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

some cli mistakes with --at lead to unexpected behavior #10

Closed
ttf opened this issue Oct 30, 2010 · 4 comments
Closed

some cli mistakes with --at lead to unexpected behavior #10

ttf opened this issue Oct 30, 2010 · 4 comments

Comments

@ttf
Copy link

ttf commented Oct 30, 2010

Today i accidentally (not for the first time though) used a different cli-format with an equal sign instead of space delimiter in timetrap:
t out --at="18 minutes ago"
It neither gracefully handled my mistake, nor thrown any error nor any other message.
t d showed: 15:55:16 - 00:00:00-304:04:44

Thanks to the t edit function this is not a big problem once spotted.

For some reason this error doesn't happen every time.
I could only reproduce it with some times like 18 minutes ago and only with the equation sign that seems to be handled sometimes better than others.

Thanks in advance!

@samg
Copy link
Owner

samg commented Oct 31, 2010

I just pushed version 1.5.0 which handles this better. Time.parse turns out to be a bit trigger happy and will convert "=18" into the 18th of the current month. I'm doing more sanity checks on the string before passing it to Time.parse if Chronic.parse fails. In the event a time can't be parsed by Chronic and won't be passed to Time (because it contains an '=' or doesn't contain a number) timetrap will report this to user, and exit with an error status code without updating the record.

@ttf
Copy link
Author

ttf commented Nov 5, 2010

Thanks! Works for me :-)

Found another (less important) one here:
t out -at"18 minutes ago"

Would be a clear user mistake to omit the space, no matter the cli format.
But might still be a nice addition to the sanity check, if it gets interpreted as -a "t18.....

@samg
Copy link
Owner

samg commented Nov 10, 2010

I've been looking into this an haven't been able to find a clear heuristic that will reject these misformatted options but won't reject valid input. For instance I could enforce non-chronic parsable times must begin with a number, but this would reject "October 12th 2pm". I could also blacklist words that would likely appear in natural language times (e.g. "ago") but blacklisting all of these seems complex, error prone, and could collide with valid times in unexpected ways.

I think this may be a case where users need to get familiar with the option parsing syntax to avoid unexpected results.

@ttf
Copy link
Author

ttf commented Nov 27, 2010

Well you already handled the problematic case. So thanks again. :-)

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

No branches or pull requests

2 participants