search

marionebl / svg-term-cli

Share terminal sessions via SVG and CSS
MIT License
3.48k stars 116 forks source link

`--from`, `--to` and `--at` using milliseconds is confusing #19

Open adius opened 6 years ago

adius commented 6 years ago

No matter what values I enter I always just get the initial view and nothing happens.

marionebl commented 6 years ago

Could you provide the commands you tried and the uploaded asciicast you are using?

marionebl commented 6 years ago

Also see https://github.com/marionebl/svg-term-cli/issues/18#issuecomment-359231989

adius commented 6 years ago

Thanks for the example in #18. Helped me to figure out my mistake. --at is in milliseconds, and the timestamps in the asciicast are in seconds. (https://github.com/asciinema/asciinema/blob/master/doc/asciicast-v1.md#frame)

Is there a way we can prevent future users make the same mistake? E.g. print a warning for small numbers and floating point numbers?

adius commented 6 years ago

I think --at should use seconds as well, but I guess it would be too much of a breaking change!?

marionebl commented 6 years ago

--at, --from and --to consistently use milliseconds as unit. This is reflected in the --help output, too.

I chose milliseconds to avoid confusion with floating point delimiters and give fine grained control. I'm leaning towards keeping it this way.

The problem with the warning you suggest is that it would trigger for legit use, e.g cases where I wanted everything in the cast, except a few frames at the start

svg-term --from=20

Expanding the time-related flag to support ms/s unit suffixes could make this a bit less confusing:

adius commented 6 years ago

I like the second idea! Allow both units and write them always explicitly in the documentation, then it should be really hard to miss!

marionebl commented 6 years ago

Jep, let's do this. Want to lend a hand?

adius commented 6 years ago

Ok. I'll open a pull request

marionebl commented 6 years ago

@adius Are you still interested in this?

adius commented 6 years ago

Yes, sorry, have to do some other things first. Finish it next week

ghost commented 6 years ago

Maybe just support the standard for time format, ISO8601/Times. More specifically, durations. Just ignore the parts about hours, days, and up.

It says seconds and fractions of a second are standard. And that both a dot and a comma are ok for a delimiter.

Actually, there must be a library function that supports the standard. No need to code this from scratch.