todo.sh replace using STDIN: remove priority prefix

[Pegasust]

Changes

• Remove the priority prefix when todo.sh replace <ID> with stdin input
• Add Cygwin little fix for t8010-listaddons.sh
  • Remove the actions that are supposedly skipped before executing the next tests

Previously

The portion surrounded by {} is the initial input provided

$ ./todo.sh add "(A) new task"
$ ./todo.sh replace 1
Replacement: {(A) new task}
1 (A) new task
TDOO: Replaced task with:
1 (A) (A) new task

After

$ ./todo.sh add "(A) new task"
$ ./todo.sh replace 1
Replacement: {new task}
1 (A) new task
TODO: Replaced task with:
1 (A) new task

Before submitting a pull request, please make sure the following is done:

• Fork the repository and create your branch from master.
• If you've added code that should be tested, add tests!
• Ensure the test suite passes.
• Format your code with ShellCheck.
• Include a human-readable description of what the pull request is trying to accomplish.
• Steps for the reviewer(s) on how they can manually QA the changes.
• Have a fixes #XX reference to the issue that this pull request fixes.

[inkarkat]

Good catch! The duplicated priority indeed is a bug.

I think that ignoring an entered priority is the wrong solution; instead, that priority should replace an existing one, like with dates. You unfortunately did not add any new tests that show the new behavior (and instead deleted and then restored an existing test, and added other unrelated stuff in the PR - please do separate submittals in the future!)

In #387, I've reworked the replacement algorithm to do a pull merge of priority + date (and added tests to show the new behavior). How do you like that?

[inkarkat]

Approach questionable; no tests added; unrelated changes.

[inkarkat]

Regarding 1a2a8ed (#386): The gitignore doesn't have any editor-specific settings; I think things like ignoring .vscode folder should go into a user-specific ~/.gitignore instead.

Regarding e4708d9 (#386): I don't understand the motivation for removing the dummy action for test skips; why is that a problem?

[Pegasust]

Thanks for reviewing my code.

Regarding the removal of the dummy action for test skips, to put it simply, the test above supposedly produces a side-effect (remove foo from list of action). The skipping mechanism doesn't produce this side-effect, while the test below it (not skipped) assumes this side-effect. So the test after it fails.

Regarding the editor-specific settings, I think your argument is valid.

I'll take a look at your PR. Meanwhile, may I ask how do you include the specific commit into these comments (like e4608d9 (#386))

[Pegasust]

Hi @inkarkat, my changes are only related to the interface given to the STDIN and are consistent with the behavior of the command. Hence, it is hard to test for what is given from read -i as it effectively pre-injects into stdin.

Before making these changes, I have also considered whether to change the behavior of todo.sh replace to also replace priority, but there seems to be a competing command to do so: todo.sh pri <num> <new-priority>.

In all honesty, I prefer your changes in #387 a lot better than the current behavior.

[inkarkat]

Regarding the removal of the dummy action for test skips, to put it simply, the test above supposedly produces a side-effect (remove foo from list of action). The skipping mechanism doesn't produce this side-effect, while the test below it (not skipped) assumes this side-effect. So the test after it fails.

Ah, I understand the problem now. Well spotted, thank you! I used to use Cygwin, but have switched to Ubuntu a long time ago; unfortunately, we still don't have a CI build on Cygwin (and embarassingly, the MacOS builds have been broken for a long time (@karbassi - any updates here?)), so this hasn't been noticed.

I've taken your commit, added some explanation and a small refactoring on top, and separately submitted that as #388.

[inkarkat]

how do you include the specific commit into these comments (like e4608d9 (#386))

Oh, that's just GitHub magic. On the /Commits\ tab on this PR, you just copy the URL to a particular commit.

[inkarkat]

In all honesty, I prefer your changes in #387 a lot better than the current behavior.

Great, thank you! If @karbassi agrees, we'll incorporate that extension of the behavior then.

With your Cygwin test fix in a separate PR (#388), there's then nothing left to be handled here, so I'm closing this PR.

Thanks for your bug report, fix, and explanations; I hope you'll enjoy todo.sh as much as we do!