search

todotxt / todo.txt-ios

Official Todo.txt iOS app for managing your todo.txt file stored in Dropbox.
http://todotxt.org
GNU General Public License v3.0
379 stars 112 forks source link

unit test fixes #216

Closed sdakin closed 10 years ago

sdakin commented 10 years ago

There were 9 failures in the unit tests that were fairly straight forward to fix.

brendonjustin commented 10 years ago

The project parser unit test failure is due to a recent change, making the project parser always parse to lowercase strings. IMO the existing test should be changed to use lowercase projects in the task string ("Check out http://example.com/this+is+a+test +research @Computer +urls" instead of "Check out http://example.com/this+is+a+test +Research @Computer +URLs"), and a new test should be added to check that the parser does parse projects to lowercase strings.

sdakin commented 10 years ago

I agree that it would be better to have more tests to cover the additional cases you mentioned. For the time being the tests have the coverage I need to move forward on the change I described in this post:

http://groups.yahoo.com/neo/groups/todotxt/conversations/topics/5128

I've got this working in my fork but it sounds like the changes for this feature will probably not want to be pulled back, based on Gina's stance regarding the official todo.txt format. It's actually a pretty minor change, as far as changes go.

ginatrapani commented 10 years ago

I've got this working in my fork but it sounds like the changes for this feature will probably not want to be pulled back, based on Gina's stance regarding the official todo.txt format. It's actually a pretty minor change, as far as changes go.

To be clear, I'm not against preserving priority on completed tasks, but I am against changing the file format. If the priority appears in completed tasks as, say, prio:A, I'd definitely merge that change.

sdakin commented 10 years ago

On the one hand it's great that the todo.txt format is flexible enough to encode a priority in more than one way, but it does lead to an obvious question. Given a simple prioritized task with this format:

(A) A prioritized task

Marking it complete currently loses the priority, but your suggestion would preserve the priority as custom metadata. Something like this:

x 2013-12-23 A prioritized task prio:A

How should a parser handle these two patterns, "(A)" in uncompleted tasks and "prio:A" in completed tasks: should they be considered identical? If so then it seems fine to put the priority in metadata for a completed task. If not then that doesn't sound like the right way to go because you would still be losing information, conceptually and semantically.

I agree that changing the file format could cause issues with existing apps, but after updating the iOS TextSplitter and Task classes to handle "(A)" patterns in completed tasks, that really does "feel" like the most correct solution. How would you feel about having a preference to support: existing behavior (delete prio on completion [default]), strict file format conformance (your prio:A suggestion), and priority priority (treat a task's priority value as a first-class format element in both completed and uncompleted tasks)?

sdakin commented 10 years ago

I pushed my changes to add support for priority in completed tasks to my fork of the todo.txt-ios repo, in case you were interested in looking at them (see previous comment for commit URL).

ginatrapani commented 10 years ago

Hi Steve - I'm not interested in changing the file format rules for a completed task, but I'm happy to merge in prio:A support in completed tasks. Priority is not as relevant in a completed task as it is in an incomplete task so it shouldn't be a first-class format element.

I know this is a controversial opinion that not everyone agrees with, but simplicity is the first and foremost consideration here. If there was strong user demand for priority in completed tasks I'd reconsider, but after 7 years of the current implementation I just haven't seen it.

sdakin commented 10 years ago

Wow, 7 years! Kudos for keeping such a useful utility going strong for that long. I understand the reluctance to change the file format - so that's out. Would you merge back a change that parses prio:A in completed tasks and interprets it as a priority value? I'm working on an app that depends on preserving priority info and I'm trying to use todo.txt for its format. If I can't I can't - but it's so close... :-)

ginatrapani commented 10 years ago

Thanks Steve.

Would you merge back a change that parses prio:A in completed tasks and interprets it as a priority value?

Absolutely. Thanks for working on the app!

ginatrapani commented 10 years ago

Belatedly: Thanks so much for the unit test fixes, Steve! I've cherry-picked those into my master.