Having to define a completion function wrapper is cumbersome. I had seen the trick of simply using ${COMP_WORDS[0]} (i.e. the used todo.sh command itself) from Paul Mansfield (https://github.com/the1ts/todo.txt-plugins/blob/develop/bash_completion/todo.txt#L7), which neatly avoids this.
By keeping the _todo_sh variable, this is a one-line change and it still allows the old way of using the override in a wrapper function. (So users aren't forced to change their customizations when upgrading.)
Tests are adapted to verify that the alias is used, and still verify the wrapper function as well.
The documentation is simplified because there's normally no need for the completion wrapper function.
This went undetected because the CI/CD pipeline was broken again due to an outdated MacOS image.
Continue to use tr for uppercasing. Enable pipefail so that grep (which needs to happen before the uppercasing) result is still considered.
(This corrects commit ca444e4000)
It can be useful to filter for non-consecutive priority ranges. By enforcing uppercase in that added syntax, overlap with general TERM(s) (e.g. "foo-bar") can be mostly avoided (and in the rare case of having to filter by all-uppercase TERM(s) one can always pass the default A-Z filter, anyway).
To indicate that something went wrong (e.g. the task already was unprioritized).
Note: For actions that handle multiple ITEMs in a loop, we cannot use die() as that would abort processing of any following ITEM(s). Instead, use a status variable and set it to 1 on error, then exit at the end.
I've seen strange readline editing behavior when the editing doesn't start at the first column: I can actually backspace into the prepended message (with Del, Ctrl-W or Ctrl-U), and then the whole edit becomes messed up.
read can output a prompt on its own (hopefully in all versions of Bash that we aim to support - the tests will tell), and that doesn't have this problem, and it's also a bit cleaner and shorter.
The prompt is only displayed if input is coming from a terminal. For the tests (currently only deletion and move confirmations are covered), this means that the prompt itself cannot be covered, and an empty line instead has to be expected. (On the positive side, this removes the ugly trick with $SPACE.)
If a custom action cannot be made non-executable, it needs to be removed as well (and the test skipped); otherwise its existence will break following tests that assume it's inactive.
So that any combination of priority / date entered in the replacement will replace the corresponding original ones, but if they are left out, the original ones will be kept.
In essence, omitted stuff will be kept, added stuff will override, only deletion of existing stuff is not possible (but this is replace, after all).
Fixes#386
This doesn't matter if (as currently recommended) the script is placed into a eagerly loaded location (like /etc/bash_completion.d/) - any name will do.
However, there's now lazy loading of completion scripts (in /usr/share/bash-completion/completions/), and that only works when the completion script is named exactly like the command the completion is for. As our command is todo.sh (ignoring aliases, which become more complex with lazy loading), the corresponding completion needs to be todo.sh (with the .sh extension) as well. Renaming does not do any harm for our recommended location, but makes it easier for users (and packagers who prepare a todo.sh package) that want to use lazy loading.
See https://github.com/todotxt/todo.txt-cli/issues/383 for the complete discussion.
I didn't know about printf's capability when I introduced quoting 10 years ago. The %q format will do the quoting, and "-v VAR" can be used to reassign to the variable.
Note: The shellquote() function has been exported for reuse by add-ons. I don't think anyone has ever used that (it was mostly intended for my own, extensive extensions, and I never used it), and is trivial to move away from, anyway.
There's no command-line option to reduce verbosity (just -v to increase it), so users who would like to remove the additional messages (cp. https://github.com/todotxt/todo.txt-cli/discussions/364) have to configure this, but the variable is hard to find.
Include the default value in commented-out form and some documentation of the possible values.
Instead of potentially falling back to the built-in action that a custom action was intended to override, but (e.g. due to file system reorganizations) now results in a broken link. The extension functionality that is then skipped may result in undesired results, but this may not be immedately obvious to the user (if the extension is not particularly verbose), so some data corruption could occur if this remains undetected.
To avoid duplicating (or somehow extracting) all the built-in actions, simply detect _any_ broken symlink; i.e. offer a superset of the required functionality. So this would also complain about a broken symlink to a non-executable custom (auxiliary) file (rarely used) if that is mistakenly passed as a custom action (unlikely).
Fixes#359
* fix whitespace
* fix spelling
* fix whitespace
* unify headers of tests
* fix some issues in tests, identified by shellcheck
* fix bash completions
bash completion files are not supposed to be executable
* fix some issues identified by shellcheck
Co-authored-by: Ali Karbassi <ali@karbassi.com>
* Tests: Add coverage for del / move without -f, but with prompting
Supplying the user confirmation via "yes".
* Cosmetics: Align inconsistent spacing for before (y/n) prompt
* Refactoring: Extract confirm() function
The user confirmation query had been duplicated (once) in the code.
* Refactoring: confirm(): Leave early if forced
* Return from user prompt without requiring Enter
By just reading a single character (y for yes, anything else: no).
* Tests: Ensure that only a single "y" concludes the confirmation
By switching from "yes" (that endlessly prints newline-separated "y"s) to "printf y".
* t1800-del: Add coverage for negative confirmation
Negative means "anything but y", so "n", "x", and Enter all apply.
* Cosmetics: Add trailing space after (y/n) prompt
So that the user's typed answer is not recorded directly after it, but with separation: "Foo? (y/n) y" instead of "Foo? (y/n)y".
*Compatibility: "read -N 1" is only available in Bash 4.1+
Mac OS still ships with Bash 3.2 :-( Fall back to the original prompting that requires conclusion via Enter then.
Note: Even though the tests use "printf y", this still gets accepted, as there'll be EOF after that. In real use (when stdin from the terminal stays open), a concluding Enter is mandatory, though.
Closes#152
When a user installs from a package manager (MacPorts, Homebrew, `yum`, etc.), the `todo.sh` script is installed into a restricted folder. This means that the result of `$(basename "$0")` is a directory where the user cannot write without elevated permissions. This generates additional noise when the user first runs `todo.sh -h`. An alternative is to default to the user's personal directory which is known to exist and will be writable by the user.
If the current directory had spaces in the directory path, this script would fail.
That has been fixed by appropriate use of quotes.
Co-authored-by: Tim Learmont <learmont@NocturnalAviation.com>
Co-authored-by: Ali Karbassi <ali@karbassi.com>
* Refactoring: Replace serial checks for `TODOTXT_CFG_FILE` and `TODO_ACTIONS_DIR` with loops
Two fallbacks (like for the actions dir) may still be fine, but we're now supporting so many config locations that a loop is much shorter, and the various locations are much easier to see.
For consistency, also apply this to the actions dir lookup, although it's less of a problem there.
* Refactoring: Apply the first default for `TODOTXT_CFG_FILE` also in the loop
There's no need to handle this separately; either an exported environment variable already exists, or it got assigned via `-d CONFIG_FILE`, or the `test -e` will fail on an empty (or bad non-existing) value, and we enter the fallback loop.
* Mention the actual config file locations in the help for `-d`
Extract the list of default locations into a `configFileLocations` array and join that into a string that then gets interpolated into the help output for `-d` (that so far only mentioned the first default location).
* Comments: Don't favor a single config location in the requirement
* Documentation: Mention `~/.todo/config` as just one of the defaults
As there are several others, now shown in the help for `-d`.
I don't want to duplicate the entire list here, as there's a high risk of those lists diverging.
Fixes#342
Uses current priority logic and option tests just running multiple times until item and priority pairs are exhausted or stops on first error.
Tests for multiple priorities, multiple re-prioritization and a couple of errors.
* TESTSFIX: t0000-config.sh may unintentionally pick up existing user config in `$XDG_CONFIG_HOME/todo/config`
This corner case was missed when the XDG alternative config location was introduced, likely because the variable isn't set by default on Ubuntu or Mac OS, and none of the developers had a config there.
HOME is already redirected to the current working directory; as `XDG_CONFIG_HOME` is just a config alternative that has a fallback to `$HOME/.config`, simply unset it.
Fixes#347
* TESTSFIX: User add-ons that override built-in actions may break tests
If `TODO_ACTIONS_DIR` is already defined, the defaulting in todo.sh are skipped. The user may have custom add-ons that override built-in actions; any incompatible behavior would negatively affect the tests then.
By setting the variable to the first default location, we avoid this interference.
* Documentation: Clarify basic regexp syntax of TERM in todo.sh list
And give hints about escaping.
Issue raised by https://github.com/todotxt/todo.txt-cli/discussions/334
* Documentation: Correct quoting in todo.sh help list
Though the double quoting doesn't do harm when the single backslash is followed by the | character (\| is no valid escape sequence), it's technically more correct to use single quotes, and also is more in line with the quoting advice given directly below it.
