todotxt / todo.txt-cli

☑️ A simple and extensible shell script for managing your todo.txt file.
http://todotxt.org
GNU General Public License v3.0
5.56k stars 713 forks source link

PRESERVE_LINE_NUMBERS option does not affect the "do" action #254

Closed pabgan closed 6 years ago

pabgan commented 6 years ago

Do you want to request a feature or report a bug? Bug

What is the current behavior? TODOTXT_PRESERVE_LINE_NUMBERS=1 does not affect the "do" action.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem.

$ t add +test 1
26 2018-05-31 +test 1
TODO: 26 added.

$ t add +test 2
27 2018-05-31 +test 2
TODO: 27 added.

$ t add +test 3
28 2018-05-31 +test 3
TODO: 28 added.

$ t ls +test
26 2018-05-31 +test 1
27 2018-05-31 +test 2
28 2018-05-31 +test 3
--
TODO: 3 of 26 tasks shown

$ t do 26
26 x 2018-05-31 2018-05-31 +test 1
TODO: 26 marked as done.
x 2018-05-31 2018-05-31 +test 1
TODO: /home/pablo/.todo-txt/todo.txt archived.

$ t do 27
27 x 2018-05-31 2018-05-31 +test 3
TODO: 27 marked as done.
x 2018-05-31 2018-05-31 +test 3
TODO: /home/pablo/.todo-txt/todo.txt archived.

What is the expected behavior? $ t do 27 "does" the task +test 2

Which versions todo.sh are you using?

Run todo.sh -V


$ t -V
TODO.TXT Command Line Interface v2.10

First release: 5/11/2006 Original conception by: Gina Trapani (http://ginatrapani.org) Contributors: http://github.com/ginatrapani/todo.txt-cli/network License: GPL, http://www.gnu.org/copyleft/gpl.html More information and mailing list at http://todotxt.com Code repository: http://github.com/ginatrapani/todo.txt-cli/tree/master


**Which Operating System are you using?**
Fedora 28

**Which version of bash are you using?**
> Run `bash --version`

$ zsh --version zsh 5.5.1 (x86_64-redhat-linux-gnu)

karbassi commented 6 years ago

Can confirm the bug.

$ cat todo.cfg
export TODO_DIR=$(dirname "$0")
export TODO_FILE="$TODO_DIR/todo.txt"
export DONE_FILE="$TODO_DIR/done.txt"
export REPORT_FILE="$TODO_DIR/report.txt"
export TODOTXT_PRESERVE_LINE_NUMBERS=1

$ todo.sh -d todo.cfg ls
--
TODO: 0 of 0 tasks shown

$ todo.sh -d todo.cfg add "task +test 1"
1 task +test 1
TODO: 1 added.

$ todo.sh -d todo.cfg add "task +test 2"
2 task +test 2
TODO: 2 added.

$ todo.sh -d todo.cfg add "task +test 3"
3 task +test 3
TODO: 3 added.

$ todo.sh -d todo.cfg add "task +test 4"
4 task +test 4
TODO: 4 added.

$ todo.sh -d todo.cfg add "task +test 5"
5 task +test 5
TODO: 5 added.

$ todo.sh -d todo.cfg ls
1 task +test 1
2 task +test 2
3 task +test 3
4 task +test 4
5 task +test 5
--
TODO: 5 of 5 tasks shown

$ todo.sh -d todo.cfg do 3
3 x 2018-05-31 task +test 3
TODO: 3 marked as done.
x 2018-05-31 task +test 3
TODO: /usr/local/bin/todo.txt archived.

$ todo.sh -d todo.cfg ls
1 task +test 1
2 task +test 2
3 task +test 4
4 task +test 5
--
TODO: 4 of 4 tasks shown

$ todo.sh -d todo.cfg do 4
4 x 2018-05-31 task +test 5
TODO: 4 marked as done.
x 2018-05-31 task +test 5
TODO: /usr/local/bin/todo.txt archived.
karbassi commented 6 years ago

I did a little digging. Not a solution, but I think I found the error.

The del/rm command does a check for TODOTXT_PRESERVE_LINE_NUMBERS and works correctly.

https://github.com/todotxt/todo.txt-cli/blob/e40e76fb07b0fcef8be6a764ca85612dfb57d1b7/todo.sh#L1120-L1130

However, when the do command is ran, it will replace the line with an x in front, which isn't the problem. The problem is the archive command running after the do command.

https://github.com/todotxt/todo.txt-cli/blob/e40e76fb07b0fcef8be6a764ca85612dfb57d1b7/todo.sh#L1205-L1209

In archive there is no check for TODOTXT_PRESERVE_LINE_NUMBERS.

https://github.com/todotxt/todo.txt-cli/blob/e40e76fb07b0fcef8be6a764ca85612dfb57d1b7/todo.sh#L1095-L1104

inkarkat commented 6 years ago

The problem is the archive command running after the do command.

Archiving is supposed to restructure / clean up the todo.txt file. Without that, the file would grow continuously, and soon consist of nothing but empty lines at the beginning. I suppose there are use cases for that (using the task / line number as an immutable, persistent, and unique ID), but todo.sh wasn't developed with this in mind.

If you follow the help closely, you'll see this behavior documented, too: TODOTXT_PRESERVE_LINE_NUMBERS is defined in terms of -n/-N behavior, and its help explicitly mentions task deletion only:

-n
    Don't preserve line numbers; automatically remove blank lines
    on task deletion
[...]
TODOTXT_PRESERVE_LINE_NUMBERS   is same as option -n (0)/-N (1)

I have written an extended archive action that runs a new defragment action; this automatically converts task references (e.g. a:42) to the changed line numbers. That's my solution for cross-references of tasks (only within todo.txt, of course).

If you really need immutable line numbers (and are willing to face the mentioned downsides), you could implement this via a custom archive overload, too. As it works as designed, I'm closing this issue.

karbassi commented 6 years ago

@inkarkat I agree with 100% of what you said.

However, shouldn't the -n flag disable the TODOTXT_AUTO_ARCHIVE option? Or maybe TODOTXT_PRESERVE_LINE_NUMBERS should be the same as -n -a? Thoughts?

pabgan commented 6 years ago

You just need to add new tasks to the first empty line instead of to the end of the file to avoid the big but empty file issue.

Couldn't this be at least an option?

inkarkat commented 6 years ago

@karbassi It's a matter of style; I actually prefer that options are independent of each other. That may require more effort in configuration, but also does not preclude any unforseen uses.

@pabgan If you allow re-use of line numbers, yes. But right now, todo.sh always appends at the end, and relies on archiving to remove empty lines. So, your suggestion would require non-trivial code changes (and add-ons have to cooperate).

The usual process is that users experiment with the tool and extend it via custom add-ons. If some functionality is very useful and sought-after by many users, it might make it into the core tool. Also, you can quickly hack together an add-on; a core addition requires documentation, test cases, and so on.

pabgan commented 6 years ago

Thanks @inkarkat . I somehow managed to get the behaviour I want aliasing the -a option and having cron run an archive every night... And adding a final filter to filter out done tasks in the meantime. Thanks!