Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue #361 - add persistence for priority on completion, as an option defaulting to original behavior #362

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix issue #361 - add persistence for priority on completion, as an option defaulting to original behavior #362

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion tests/t0000-config.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ test_expect_success 'no config file' '

# All the below tests will output the usage message.
cat > expect <<EOF
Usage: todo.sh [-fhpantvV] [-d todo_config] action [task_number] [task_description]
Usage: todo.sh [-fhpamntvV] [-d todo_config] action [task_number] [task_description]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder whether this needs to be exposed as a command-line option - it struck me as noteworthy that most of your proposed change is about these boilerplate modifications, and the actual extension is just a few lines.

I guess most users would like to permanently configure TODOTXT_PRESERVE_PRIORITY=1 in the config file, anyway. (The same argument applies to the existing auto-archive, preserve line-numbers, and date prepending; unfortunately, these already have set the precedent in favor of short options, and it's getting increasingly more difficult to find good candidate letters.)

For my own scripts, I'd just offer a long GNU-style command-line option a la --keep-priority-on-done for this. But this would mean switching to GNU getopt (which is a separate dependency on Mac OS), and many adaptations. (And I wouldn't expect you to do this as part of this PR - this is just me brainstorming for the co-developer.)

Try 'todo.sh -h' for more information.
EOF

Expand Down
8 changes: 4 additions & 4 deletions tests/t2100-help.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,21 +11,21 @@ This test covers the help output.
# slightly changes, only check for the section headers.
test_todo_session 'help output' <<EOF
>>> todo.sh help | sed '/^ [A-Z]/!d'
Usage: todo.sh [-fhpantvV] [-d todo_config] action [task_number] [task_description]
Usage: todo.sh [-fhpamntvV] [-d todo_config] action [task_number] [task_description]
Options:
Built-in Actions:
EOF

test_todo_session 'verbose help output' <<EOF
>>> todo.sh -v help | sed '/^ [A-Z]/!d'
Usage: todo.sh [-fhpantvV] [-d todo_config] action [task_number] [task_description]
Usage: todo.sh [-fhpamntvV] [-d todo_config] action [task_number] [task_description]
Options:
Built-in Actions:
EOF

test_todo_session 'very verbose help output' <<EOF
>>> todo.sh -vv help | sed '/^ [A-Z]/!d'
Usage: todo.sh [-fhpantvV] [-d todo_config] action [task_number] [task_description]
Usage: todo.sh [-fhpamntvV] [-d todo_config] action [task_number] [task_description]
Options:
Environment variables:
Built-in Actions:
Expand All @@ -34,7 +34,7 @@ EOF
make_action "foo"
test_todo_session 'help output with custom action' <<EOF
>>> todo.sh -v help | sed '/^ [A-Z]/!d'
Usage: todo.sh [-fhpantvV] [-d todo_config] action [task_number] [task_description]
Usage: todo.sh [-fhpamntvV] [-d todo_config] action [task_number] [task_description]
Options:
Built-in Actions:
Add-on Actions:
Expand Down
12 changes: 6 additions & 6 deletions tests/t2120-shorthelp.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,15 @@ This test covers the output of the -h option and the shorthelp action.
# check for the section headers.
test_todo_session '-h output' <<EOF
>>> todo.sh -h | sed '/^ [A-Z]/!d'
Usage: todo.sh [-fhpantvV] [-d todo_config] action [task_number] [task_description]
Usage: todo.sh [-fhpamntvV] [-d todo_config] action [task_number] [task_description]
Actions:
Actions can be added and overridden using scripts in the actions
See "help" for more details.
EOF

test_todo_session 'shorthelp output' <<EOF
>>> todo.sh shorthelp | sed '/^ [A-Z]/!d'
Usage: todo.sh [-fhpantvV] [-d todo_config] action [task_number] [task_description]
Usage: todo.sh [-fhpamntvV] [-d todo_config] action [task_number] [task_description]
Actions:
Actions can be added and overridden using scripts in the actions
See "help" for more details.
Expand All @@ -28,7 +28,7 @@ EOF
make_action "foo"
test_todo_session 'shorthelp output with custom action' <<EOF
>>> todo.sh -v shorthelp | sed '/^ [A-Z]/!d'
Usage: todo.sh [-fhpantvV] [-d todo_config] action [task_number] [task_description]
Usage: todo.sh [-fhpamntvV] [-d todo_config] action [task_number] [task_description]
Actions:
Actions can be added and overridden using scripts in the actions
Add-on Actions:
Expand All @@ -48,7 +48,7 @@ export TODOTXT_GLOBAL_CFG_FILE=global.cfg

test_todo_session '-h and fatal error without config' <<EOF
>>> todo.sh -h | sed '/^ \\{0,2\\}[A-Z]/!d'
Usage: todo.sh [-fhpantvV] [-d todo_config] action [task_number] [task_description]
Usage: todo.sh [-fhpamntvV] [-d todo_config] action [task_number] [task_description]
Actions:
Actions can be added and overridden using scripts in the actions
See "help" for more details.
Expand All @@ -59,7 +59,7 @@ EOF
# Config option comes too late; "Add-on Actions" is *not* mentioned here.
test_todo_session '-h and fatal error with trailing custom config' <<EOF
>>> todo.sh -h -d custom.cfg | sed '/^ \\{0,2\\}[A-Z]/!d'
Usage: todo.sh [-fhpantvV] [-d todo_config] action [task_number] [task_description]
Usage: todo.sh [-fhpamntvV] [-d todo_config] action [task_number] [task_description]
Actions:
Actions can be added and overridden using scripts in the actions
See "help" for more details.
Expand All @@ -70,7 +70,7 @@ EOF
# Config option processed; "Add-on Actions" is mentioned here.
test_todo_session '-h output with preceding custom config' <<EOF
>>> todo.sh -d custom.cfg -h | sed '/^ \\{0,2\\}[A-Z]/!d'
Usage: todo.sh [-fhpantvV] [-d todo_config] action [task_number] [task_description]
Usage: todo.sh [-fhpamntvV] [-d todo_config] action [task_number] [task_description]
Actions:
Actions can be added and overridden using scripts in the actions
Add-on Actions:
Expand Down
2 changes: 1 addition & 1 deletion tests/t6000-completion.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ This test checks basic todo_completion of actions and options
. ./test-lib.sh

readonly ACTIONS='add a addto addm append app archive command del rm depri dp do help list ls listaddons listall lsa listcon lsc listfile lf listpri lsp listproj lsprj move mv prepend prep pri p replace report shorthelp'
readonly OPTIONS='-@ -@@ -+ -++ -d -f -h -p -P -PP -a -n -t -v -vv -V -x'
readonly OPTIONS='-@ -@@ -+ -++ -d -f -h -p -P -PP -a -m -n -t -v -vv -V -x'

test_todo_completion 'all arguments' 'todo.sh ' "$ACTIONS $OPTIONS"
test_todo_completion 'arguments beginning with a' 'todo.sh a' 'add a addto addm append app archive'
Expand Down
2 changes: 1 addition & 1 deletion tests/t6050-completion-addons.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ This test checks todo_completion of custom actions in .todo.actions.d
. ./test-lib.sh

readonly ACTIONS='add a addto addm append app archive command del rm depri dp do help list ls listaddons listall lsa listcon lsc listfile lf listpri lsp listproj lsprj move mv prepend prep pri p replace report shorthelp'
readonly OPTIONS='-@ -@@ -+ -++ -d -f -h -p -P -PP -a -n -t -v -vv -V -x'
readonly OPTIONS='-@ -@@ -+ -++ -d -f -h -p -P -PP -a -m -n -t -v -vv -V -x'

readonly ADDONS='bar baz foobar'

Expand Down
2 changes: 1 addition & 1 deletion tests/t8000-actions.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ if [ -x .todo.actions.d/foo ]; then
fi
test_todo_session 'nonexecutable action' <<EOF
>>> todo.sh foo
Usage: todo.sh [-fhpantvV] [-d todo_config] action [task_number] [task_description]
Usage: todo.sh [-fhpamntvV] [-d todo_config] action [task_number] [task_description]
Try 'todo.sh -h' for more information.
=== 1
EOF
Expand Down
2 changes: 1 addition & 1 deletion tests/t9999-testsuite_example.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ TODO: 6 of 6 tasks shown
TODO: 6 of 6 tasks shown

>>> todo.sh remdup
Usage: todo.sh [-fhpantvV] [-d todo_config] action [task_number] [task_description]
Usage: todo.sh [-fhpamntvV] [-d todo_config] action [task_number] [task_description]
Try 'todo.sh -h' for more information.
=== 1
EOF
Expand Down
19 changes: 15 additions & 4 deletions todo.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ TODO_SH=$(basename "$0")
TODO_FULL_SH="$0"
export TODO_SH TODO_FULL_SH

oneline_usage="$TODO_SH [-fhpantvV] [-d todo_config] action [task_number] [task_description]"
oneline_usage="$TODO_SH [-fhpamntvV] [-d todo_config] action [task_number] [task_description]"

usage()
{
Expand Down Expand Up @@ -113,6 +113,8 @@ $indentedJoinedConfigFileLocations
Don't auto-archive tasks automatically on completion
-A
Auto-archive tasks automatically on completion
-m
Maintain priority field on completion
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the "maintain" is meant to establish a mnemonic for the chosen option letter, but it sounds a bit confusing to my non-native ears. I'd prefer Don't remove a priority on completion.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, on both points.

-n
Don't preserve line numbers; automatically remove blank lines
on task deletion
Expand Down Expand Up @@ -142,6 +144,7 @@ $indentedJoinedConfigFileLocations
TODOTXT_CFG_FILE=CONFIG_FILE is same as option -d CONFIG_FILE
TODOTXT_FORCE=1 is same as option -f
TODOTXT_PRESERVE_LINE_NUMBERS is same as option -n (0)/-N (1)
TODOTXT_PRESERVE_PRIORITY=1 is same as option -m
TODOTXT_PLAIN is same as option -p (1)/-c (0)
TODOTXT_DATE_ON_ADD is same as option -t (1)/-T (0)
TODOTXT_PRIORITY_ON_ADD=pri default priority A-Z
Expand Down Expand Up @@ -510,6 +513,7 @@ uppercasePriority()
#Preserving environment variables so they don't get clobbered by the config file
OVR_TODOTXT_AUTO_ARCHIVE="$TODOTXT_AUTO_ARCHIVE"
OVR_TODOTXT_FORCE="$TODOTXT_FORCE"
OVR_TODOTXT_PRESERVE_PRIORITY="$TODOTXT_PRESERVE_PRIORITY"
OVR_TODOTXT_PRESERVE_LINE_NUMBERS="$TODOTXT_PRESERVE_LINE_NUMBERS"
OVR_TODOTXT_PLAIN="$TODOTXT_PLAIN"
OVR_TODOTXT_DATE_ON_ADD="$TODOTXT_DATE_ON_ADD"
Expand All @@ -524,7 +528,7 @@ OVR_TODOTXT_FINAL_FILTER="$TODOTXT_FINAL_FILTER"
export GREP_OPTIONS=""

# == PROCESS OPTIONS ==
while getopts ":fhpcnNaAtTvVx+@Pd:" Option
while getopts ":fhpcmnNaAtTvVx+@Pd:" Option
do
case $Option in
'@')
Expand Down Expand Up @@ -581,7 +585,10 @@ do
set -- '-h' 'shorthelp'
OPTIND=2
;;
n)
m )
OVR_TODOTXT_PRESERVE_PRIORITY=1
;;
n )
OVR_TODOTXT_PRESERVE_LINE_NUMBERS=0
;;
N)
Expand Down Expand Up @@ -632,6 +639,7 @@ shift $((OPTIND - 1))
TODOTXT_VERBOSE=${TODOTXT_VERBOSE:-1}
TODOTXT_PLAIN=${TODOTXT_PLAIN:-0}
TODOTXT_FORCE=${TODOTXT_FORCE:-0}
TODOTXT_PRESERVE_PRIORITY=${TODOTXT_PRESERVE_PRIORITY:-0}
TODOTXT_PRESERVE_LINE_NUMBERS=${TODOTXT_PRESERVE_LINE_NUMBERS:-1}
TODOTXT_AUTO_ARCHIVE=${TODOTXT_AUTO_ARCHIVE:-1}
TODOTXT_DATE_ON_ADD=${TODOTXT_DATE_ON_ADD:-0}
Expand Down Expand Up @@ -738,6 +746,9 @@ fi
if [ -n "$OVR_TODOTXT_FORCE" ] ; then
TODOTXT_FORCE="$OVR_TODOTXT_FORCE"
fi
if [ -n "$OVR_TODOTXT_PRESERVE_PRIORITY" ] ; then
TODOTXT_PRESERVE_PRIORITY="$OVR_TODOTXT_PRESERVE_PRIORITY"
fi
if [ -n "$OVR_TODOTXT_PRESERVE_LINE_NUMBERS" ] ; then
TODOTXT_PRESERVE_LINE_NUMBERS="$OVR_TODOTXT_PRESERVE_LINE_NUMBERS"
fi
Expand Down Expand Up @@ -1231,7 +1242,7 @@ case $action in
if [ "${todo:0:2}" != "x " ]; then
now=$(date '+%Y-%m-%d')
# remove priority once item is done
sed -i.bak "${item}s/^(.) //" "$TODO_FILE"
[[ "$TODOTXT_PRESERVE_PRIORITY" = "1" ]] || sed -i.bak $item"s/^(.) //" "$TODO_FILE"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those purely cosmetic, I'd prefer to keep the variable $item inside the double quotes as "${item}s/^(.) //". My shellcheck isn't clever enough to figure out that the word-split variable contents can safely be kept outside quotes.

sed -i.bak "${item}s|^|x $now |" "$TODO_FILE"
if [ "$TODOTXT_VERBOSE" -gt 0 ]; then
getNewtodo "$item"
Expand Down
2 changes: 1 addition & 1 deletion todo_completion
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ _todo()
cur="${COMP_WORDS[COMP_CWORD]}"
prev="${COMP_WORDS[COMP_CWORD-1]}"

local -r OPTS="-@ -@@ -+ -++ -d -f -h -p -P -PP -a -n -t -v -vv -V -x"
local -r OPTS="-@ -@@ -+ -++ -d -f -h -p -P -PP -a -m -n -t -v -vv -V -x"
local -r COMMANDS="\
add a addto addm append app archive command del \
rm depri dp do help list ls listaddons listall lsa listcon \
Expand Down