Completion date can't exist without creation date

[alexesmet]

Even though that pircture that explains the file format for todo.txt says that creation date must be specified when completion date is, further documentation explains that creation date is only required if you would like to count how many days have you been doing the task. Other tools also allow specifying completion date without creation date.

The problem is that ttdl does not respect completion dates without creation dates. When modifying todo.txt file contents, it erases completion date if there is no creation date. It also does not add completion date to tasks if they did not have creation date. todo.sh, in contrast, adds completion dates even if there are no creation dates.

I would be happy to take part in fixing this issue, but I would like to hear maintainers and authors thoughts on whether they believe it is possible to implement, and is that required at all.

[VladimirMarkelov]

Thank you for pointing at the issue. It seems I made the "feature" by intention. But at this moment, I cannot say "why".

Now we should decided what we are going to improve. Definitely, todo_lib crate must be patched, ttdl probably would remain intact. It depends on what we want to fix. Below my thoughts about it:

Minimal fix: do not erase completion date if it exists. To do this, it seems, it is enough to change impl std::fmt::Display for Task in file todotxt/task.rs. There completion date is saved only if creation date is not None.

Bigger fix: in addition to "minimal fix", allow TTDL to set completion date even if creation date is not set. It is done in the same file, method Task::complete. Though, it is not sufficient. As nobody complains about the current TTDL behavior, I guess, majority of people are fin with it. So, I would like to keep this behavior by default. To do this, we probably have to introduce a new tunable (in addition to existing tunable creation_date_auto) that defines whether TTDL sets completion date when a task is done. Probably, it should have only two values of possible three ones(I describe them, but they need some understandable and concise names): "set completion date if creation date exists"(the current behavior) and "set completion date always". The rest value "never set completion date" seems to me kind of useless, but , maybe, it would be useful to someone. The new tunable should be also passed to Task::complete like CompletionMode.

The bigger fix requires creating patches for both library todo_lib and to the binary ttdl.

Extra point: while todo_lib lacks some tests, I tend to add tests every time I introduce a new feature. So, it would be great if we also add tests (at least four of them: parsing, displaying, complete, and uncomplete a task with completion date only) to todo_lib. No need to add tests to ttdl.

What do you think about it?

[VladimirMarkelov]

Note: I've just update the code, so please rebase your clone if you are going to do anything. I noticed a few compiler warnings in CI and fixed them along with updating thirdparty crates.

[alexesmet]

I think that "minimal" and "bigger" fix could be two separate merge requests, both of which I'm going to start working soon (so you can assign the issue to me).

So, I would like to keep this behavior by default.

I totally agree that we should not change the default behavior, so looks like we will need a new tunable. If you are fine with being a bit verbose, we could call it add_completion_date_without_creation_date or something (help with naming would be appreciated)

The rest value "never set completion date" seems to me kind of useless, but , maybe, it would be useful to someone.

I was thinking about covering the "never set completion date" usecase by making the AddCompletionDateTunable an enum with Always, CreationDatePresent, and Never variants, but looks like Never variant would not be possible because of todo.txt syntax - it will interpret first date of a completed task as a completion date, so the only way for a completed task to preserve creation date is to have completion date. We are left with two variants, so a bool should be enough.

So, it would be great if we also add tests

I admire TDD, I'll add tests for these usecases :+1:

I'm planning to start work on this on the next week. Special thanks for providing technical clues

[VladimirMarkelov]

I think that "minimal" and "bigger" fix could be two separate merge requests.

It makes sense. As I see, both features do not break any existing workflow, so there is no need to increase the major version number. I think, after merging both MRs we can increase minor versions of todo_lib and ttdl and then publish it to crates.io.

Yes, it is was my thought as well: Never makes parsing ambiguous. That was why I was unsure and said "probably". As you agree with my doubt, let's ignore the Never case 😄 As of naming, maybe use a bit shorter add_completion_date_always. In this case, default value for boolean - false - keeps the TTDL behavior the same as it does now. We should add a good comment to example configuration ttdl.yaml with explanation when to set it to true.

Thank you! I'm looking forward to your MRs!

[alexesmet]

As far as I understand, we now need to wait when it is time to publish a new version of todo_lib before we can add this functionality into ttdl. If you notice me miss this moment, please let me know, and I'll continue moving this feature forward.

[VladimirMarkelov]

Yes, we have to wait. I'll publish it a bit later.

[VladimirMarkelov]

Version 7.2.0 of todo_lib published

[VladimirMarkelov]

TTDL 4.4.0 published on crates.io and new binaries are provided in release section.