-
Notifications
You must be signed in to change notification settings - Fork 93
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
Starting tracker with single-comma delimited description doesn't work (with hamster 3.0.2) #334
Comments
If we want to fix this in hamster, we should probably discuss it there, I created projecthamster/hamster#657 for that. |
@rhertzog This change was not done lightly. But you are right that the extension should follow, see my recommendations: |
@ederag I see that you have thought hard to support quite some corner case but I believe it's not the good trade off that you picked. |
@rhertzog No time to dive into my notes, but it was needed. The "corner cases" came as a bonus. |
I can just repeat what I said before: the current rules of the "permissive parser" are overly complex. Too much complexity causes confusion, and the "double comma" ist just one particularly weird example for that. It has all been done with good intentions, trying to satisfy user requests. Yet I believe it was misguided. Simplicity is key, the syntax rules have to be so simple that they can be understood at a single glance. IMO for the shell command line, they should match what command line users are used to (and that means: no whitespace in activity and category names). But I'm digressing and probably overdoing it. The dilemma we have now is that in a way, the ship has sailed - 3.x has been out in the field with these parsing rules for almost a year. If we revert this now, we'll make some happy, and others who've come to like the new rules will be upset. We have no metrics to judge how far this adoption has gone yet, and what the final thumbs-up/down relation will be. But we need to choose one. Going for some compromise with yet new rules would confuse people even more. My personal vote, no surprise here, is to revert this change. |
I'd suggest all discussion about reverting the change happen in projecthamster/hamster#657, so that everything is in the same place. I'd suggest this issue is either fixed for the current released version, or is suspended until projecthamster/hamster#657 is resolved. |
Agreed, the technical discussion should go to projecthamster/hamster#657,
True. |
Clearly we don't have the same definition of user-friendly. And if instead of laughing at my code, you'd have taken the time to read it, you would see that my changes actually make the parser more robust while not introducing the needless complexity of the required double-comma.
This is not true. If anyone uses double-comma, it will continue to work as usual with a single-comma as delimiter. We can even add some tests to ensure we properly strip the extra comma. |
I did not. And I also disagree with the remainder (you're missing the point). |
This was fixed with merging projecthamster/hamster#663, so I think this issue can be closed. |
Go for closing then. If this is an error, we can reopen it on request. (ping @rhertzog) |
I'm astonished that you chose to break the v3 parser, |
With hamster 3.0.2, if you type
Activity@Category, task description
in the shell extension, nothing happens. To get it to actually start, you need to typeActivity@Category,, task description
. This is likely related to the breaking change made in hamster that now requires two commas to separate descriptions:projecthamster/hamster@42d3c05
I think we should revisit that decision as it doesn't seem like a good decision to me. In particular when typing
Activity@Category, #tag
just works.But if we don't, then we somehow need to fix the shell extension. Something needs to happen when the user inputs something... either it creates an activity with some unexpected values, or it displays an error. But just dropping the user's input is wrong.
The text was updated successfully, but these errors were encountered: