Skip to content
This repository has been archived by the owner on Sep 17, 2018. It is now read-only.

Support for providing file instead of url #24

Open
reggi opened this issue May 5, 2016 · 15 comments
Open

Support for providing file instead of url #24

reggi opened this issue May 5, 2016 · 15 comments

Comments

@reggi
Copy link

reggi commented May 5, 2016

Here I get Wrong URL format

asciinema2gif --size small --speed 2 -o "${HOME}/Desktop/another.gif" /Users/thomasreggi/Desktop/example.json 
@vitorgalvao
Copy link
Contributor

asciinema2gif, after you give it a url, basically goes to the website and takes a bunch screenshots, stitching them together.

What this means is that it is in no way prepared for to take a json and make it a gif. Nor could it, without taking code from asciinema itself.

I’d say this is really out-of-scope.

@techtonik
Copy link

Why not add asciinema as an optional dependency for this kind of stuff? It could be pretty useful.

@vitorgalvao
Copy link
Contributor

@techtonik Because, as I said, this tool is absolutely not prepared for the requested task. Requiring asciinema won’t do anything by itself. To perform the requested action isn’t to add an option, is to make a wholy different tool.

@techtonik
Copy link

techtonik commented May 7, 2016

@vitorgalvao you probably want to say that asciinema requires some VM to execute javascript player, according to this page - https://asciinema.org/docs/how-it-works - so it should be two dependencies.

As for if the tool for doing that should be different, because of implementation, I disagree. Ask 5 people how do they think what tool named asciinema2gif should do prove.

@vitorgalvao
Copy link
Contributor

vitorgalvao commented May 7, 2016

you probably want to say that asciinema requires some VM to execute javascript player, according to this page - https://asciinema.org/docs/how-it-works - so it should be two dependencies.

My comment was only meant to clarify that dependencies exist, and they might not be trivial. It didn’t try to be accurate on what those are.

Ask 5 people how do they think what tool named asciinema2gif should do prove.

That is absolutely irrelevant. The name of a tool does not give anyone the right to demand features1.

The fact is introducing this new feature is as much trouble as making a completely new tool, due to the current implementation. My sole point with my previous comments was to clarify this and set expectations accordingly.

Since the addition is not trivial and out of the original scope of the tool, it is unlikely any of the current contributors will spend their time on it (if they wanted the feature, they would have likely already started working on it).

In sum, the only thing I meant by my previous remarks is I doubt this is going to be implemented without a pull request.


1 To be clear, I do not think any of you is demanding anything or being unreasonable.

@techtonik
Copy link

There is only one thing with your comments - there are some facts that only you know, but without any proof, it is hard to agree to your arguments. Instead of setting the expectations, the more constructive strategy is describing why something is unwieldy and hard to fix. Because there might be something that you've missed.

asciinema2gif already requires fatty phantomjs2 and imagemagick, so I don't see any reason why it could not work with packaged asciinema and why it should be a separate tool. The arguments due to the current implementation and the addition is not trivial and out of the original scope of the tool are not convincing, because they are not backed up by facts.

@vitorgalvao
Copy link
Contributor

vitorgalvao commented May 11, 2016

The proof is the source code. If you read it and understand it, there it is. And it’s a pretty small codebase.

My facts and arguments are backed up by my extensive contributions to the tool. Those were all pretty recent; I know the main script and its current goals very well (I’ve also worked on the documentation, when altering/adding features), and the secondary one is a helper to the first.

There’s no reason for you to get riled up. The fact (backed up by my knowledge of the tool and extensive open-source contributions with both small tools and managing big ones, which are all easily verifiable by you), is the tool is currently geared towards a single kind of action, and introducing that new functionality is a decent enough departure from its current capabilities that there is a good chance (emphasis on the word “chance”) the people that work on this tool (including me) will not work for free on that addition if they do not want it themselves. This means you shouldn’t be holding your breath for this, and have your expectations of this issue being resolved in the near future (or at all) set accordingly.

@techtonik
Copy link

@vitorgalvao it would be more useful for someone willing to spend time on PR if you could describe technical challenges the lie ahead in the same detail you describe your experience. =)

@vitorgalvao
Copy link
Contributor

@techtonik Please don’t try turning this around. Quite frankly, I’m getting tired of such an unproductive discussion.

I’ve spent the majority of this issue explaining why this might not be easily feasible/implemented. I’ve only mentioned my experience after you actively questioned it and my arguments, claiming I could not prove them and questioning their validity and factuality.

It’s becoming abundantly clear you’re more concerned with having the last word than actually understanding the problem.

What would be more useful would be for you to do your own research and ask specific questions, rather than arguing without context and contradicting my points with no knowledge of the tool’s limitations, even after I explained the difficulties.

I have no need for this feature, and do not intend to spend my free time working on it for free and researching its implementation for you. I have no problem helping users and even fixing things that aren’t working as they expect, even if I won’t necessarily use them, but you need to respect other people’s time. Do some actual work and ask specific questions. Otherwise, I have nothing else to discuss with you.

If you still cannot understand why this isn’t a trivial change and why that means I though it was important to manage @reggi’s expectations that this might not actually get done, then you’re beyond my help. I believe I have been pretty clear, and have no desire to keep wasting my time with this.

In fact, lets put the discussion to rest with my next reply.

@vitorgalvao
Copy link
Contributor

@wong2 @tav If you don’t mind, do any of you:

  • Plan on implementing this.
  • Do not plan on implementing it, but are willing to look at a pull request if it is well made.
  • Feel this is out of scope for the tool for now.

If it’s the second or third option, it might be better to just close the issue. Even if you’re willing to accept a PR, having the lingering open issue will not make it happen by itself.

@ku1ik
Copy link

ku1ik commented May 11, 2016

Hey guys, asciinema dev here.

Ideally, I would see asciinema-to-gif conversion implemented as I described in my comment here asciinema/asciinema-player#24 (comment) . However, asciinema2gif is doing the job, and it's here, right now, so thanks for your work on it!

Anyway, here's a quick, from the top of my head, idea on how this tool could support JSON files directly.

Right now asciinema2gif opens the given URL in PhantomJS browser. This URL points to a simple .html page, which links to asciinema player's .js file and recording .json file. Both .html and .js are static so they actually could be embedded in asciinema2gif (.html page needs to initialize the player with proper URL and options but that could be solved by embedding a template of this page, or calling some static js code on this page with URL as arg, from PhantomJS context). So the only variable part would the .json file which could come from local filesystem or could be fetched from asciinema.org (every asciinema.org page like https://asciinema.org/a/1234 uses .json file at https://asciinema.org/a/1234.json).

I'm not saying it's trivial and not time consuming, but I think this could be the quickest route to achieve what is discussed here (if anyone considered working on it).

@ku1ik
Copy link

ku1ik commented May 11, 2016

Note on using asciinema as an (optional) dependency.

If we talk about asciinema command line recorder, then it wouldn't help as it's dumb recorder that has nothing to do with the web player. I don't really plan to add native gif generation (nor to embed the player) to asciinema binary.

If we talk about asciinema player, then it makes sense. Right now the player is used by asciinema2gif, but indirectly, through asciinema.org website. Both solutions I described in my prev comment (the one I imagined in my linked comment and the one I described above) would make the player a direct dependency.

@jfmherokiller
Copy link
Contributor

well i think it might be possible todo it with only phantomjs but i think the render script needs to be edited to be able to accept this template html file i created here

@calvn
Copy link

calvn commented Jan 16, 2017

Maybe functionality from https://github.com/pettarin/asciicast2gif could be merged into here. It seems that the only component missing is serving the JSON file locally to enable this.

@ku1ik
Copy link

ku1ik commented May 1, 2017

Hey! I built https://github.com/asciinema/asciicast2gif which supports both local files and remote URLs. You may want to check it out.

I planned to base my work on either asciinema2gif or pettarin/asciicast2gif, and send PRs with the improvements, however it turned out my idea of approaching GIF generation problem required major rethink and it was much, much closer to what asciinema already had for PNG preview generation (I based asciicast2gif on a2png codebase). In short: not making screenshots at constant frame-rate, instead make them only when terminal contents change, by using asciinema/asciinema-player's terminal emulator.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants