mpcjanssen / simpletask-android

GNU General Public License v3.0
548 stars 128 forks source link

Adding text after due date sends item to the end of the date sorted list #849

Open Piscean opened 6 years ago

Piscean commented 6 years ago

Repro:

  1. Sort your list so items due today are at the top of the list
  2. Edit the top item in the list
  3. Cursor is placed at the start of the line.
  4. Rather than insert text before the due date, I just moved to after the due date and started typing --> The keyboard automatically dropped a '.' at the end of the date before it started adding my text --> Simpletask puts "Work item due:2018-01-03. With added text." at the very end of the sorted list.

"due:2018-01-03." should sort the same as "due:2018-0103"

smichel17 commented 6 years ago

If you have Work item due:2018-01-03 With added text., it will sort "correctly". The period makes it sort "incorrectly" because the due date is no longer recognised, so it's sorted as if it has no due date.

Quotes because I believe the spec says the date must be followed by a space, making this intended behavior. I do think it would be reasonable to change, though, so that trailing punctuation does not invalidate the date (I think trailing letters still should); ultimately it's @mpcjanssen's decision.

Piscean commented 6 years ago

Oh, interesting. I haven't read the spec, but if it's working the way it's supposed to, then not to worry. It's just different behavior from the Windows todotxt.net app so I thought I'd flag it.

smichel17 commented 6 years ago

Technically, due: is an extension of the spec, which includes a section on how extensions are defined (using key:value pairs). So, our due: extension can have whatever behavior it wants and still be compliant.

However, the original todo.sh tool included support for addons, and extensions supported by those addons (including due:) have become de facto standards, so if we did allow trailing punctuation, other tools would likely not be able to parse the date.

Thus, either decision is reasonable. My personal preference is not to be held back by a legacy implementation (especially since supporting this does not mean we should ever intentionally place a period after the date, only that we should still parse it if one exists, although that does entail some weird stuff in how we handle this in lua). But, compatibility is also a good thing, so I'll stand by whatever decision @mpcjanssen makes.

mpcjanssen commented 6 years ago

I would prefer to accept 'obvious' stuff like due:20171212. but to make sure to never emit such a thing when using the UI to set dates. I will take this into account when redoing Task.kt

mpcjanssen commented 6 years ago

There is no real reason to check the character after any date as long as the date is YYYY-MM-DD. Will change this behavior.

mpcjanssen commented 6 years ago

Hmm, less easy than expected because of the way the parser tokenizes.

Piscean commented 6 years ago

Then close this issue. The ending '.' is caused by the keyboard, and was only because I went back to edit something. It's not a common case, and no data was lost. (I just couldn't find it.)

mpcjanssen commented 6 years ago

@Piscean I still would like this to work so not closing the issue yet. Just handling it with lower prio.