Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
adius opened this issue Jan 21, 2018 · 11 comments
Open

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

adius opened this issue Jan 21, 2018 · 11 comments

Comments

@adius
Copy link

adius commented Jan 21, 2018

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

@marionebl
Copy link
Owner

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

@marionebl
Copy link
Owner

Also see #18 (comment)

@adius
Copy link
Author

adius commented Jan 21, 2018

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
Copy link
Author

adius commented Jan 21, 2018

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

@marionebl
Copy link
Owner

--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:

  • svg-term --at=10000 => 10s / 10000ms
  • svg-term --at=10000ms => 10s / 10000ms
  • svg-term --at=10s => 10s / 10000ms

@adius
Copy link
Author

adius commented Jan 21, 2018

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 marionebl changed the title --from, --to and --at don't seem to work --from, --to and --at using milliseconds is confusing Jan 21, 2018
@marionebl
Copy link
Owner

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

@adius
Copy link
Author

adius commented Jan 21, 2018

Ok. I'll open a pull request

@marionebl
Copy link
Owner

@adius Are you still interested in this?

@adius
Copy link
Author

adius commented Jan 27, 2018

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

@ghost
Copy link

ghost commented Mar 20, 2018

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.

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

No branches or pull requests

2 participants