From 803881998fed3b34a0f219bd28a03d247183c204 Mon Sep 17 00:00:00 2001 From: Ingo Karkat Date: Sat, 21 Jan 2023 18:59:32 +0100 Subject: [PATCH 1/2] FIX: Use standard error for die() and dieWithHelp() By convention, error output should be printed to standard error, not standard out. Same for the usage help that may accompany the error message. --- todo.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/todo.sh b/todo.sh index fb8d4bc..49d1655 100755 --- a/todo.sh +++ b/todo.sh @@ -349,14 +349,14 @@ dieWithHelp() case "$1" in help) help;; shorthelp) shorthelp;; - esac + esac >&2 shift die "$@" } die() { - echo "$*" + echo >&2 "$*" exit 1 } From ef419f3594c79f329f7f156ade9a2a767a58ee62 Mon Sep 17 00:00:00 2001 From: Ingo Karkat Date: Sat, 21 Jan 2023 19:01:24 +0100 Subject: [PATCH 2/2] Use die() / print to stderr for error conditions 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. --- tests/t1200-pri.sh | 1 + tests/t1500-do.sh | 1 + tests/t1700-depri.sh | 1 + tests/t1800-del.sh | 3 +++ tests/t1910-deduplicate.sh | 1 + tests/t2120-shorthelp.sh | 6 +++--- todo.sh | 23 ++++++++++++++++------- 7 files changed, 26 insertions(+), 10 deletions(-) diff --git a/tests/t1200-pri.sh b/tests/t1200-pri.sh index fd8bd10..54b610f 100755 --- a/tests/t1200-pri.sh +++ b/tests/t1200-pri.sh @@ -90,6 +90,7 @@ TODO: 2 re-prioritized from (C) to (A). TODO: 3 of 3 tasks shown >>> todo.sh pri 2 a +=== 1 2 (A) notice the sunflowers TODO: 2 already prioritized (A). diff --git a/tests/t1500-do.sh b/tests/t1500-do.sh index 7999498..d359a49 100755 --- a/tests/t1500-do.sh +++ b/tests/t1500-do.sh @@ -81,6 +81,7 @@ test_todo_session 'fail multiple do attempts' <>> todo.sh -a do 3 +=== 1 TODO: 3 is already marked done. EOF diff --git a/tests/t1700-depri.sh b/tests/t1700-depri.sh index 7ec7e7f..f459ef1 100755 --- a/tests/t1700-depri.sh +++ b/tests/t1700-depri.sh @@ -82,6 +82,7 @@ test_todo_session 'depriority of unprioritized task' <>> todo.sh depri 3 2 +=== 1 TODO: 3 is not prioritized. 2 notice the sunflowers TODO: 2 deprioritized. diff --git a/tests/t1800-del.sh b/tests/t1800-del.sh index 6d96e89..bdf6670 100755 --- a/tests/t1800-del.sh +++ b/tests/t1800-del.sh @@ -60,6 +60,7 @@ test_todo_session 'del with confirmation' <>> printf n | todo.sh del 1 +=== 1 Delete '(B) smell the uppercase Roses +flowers @outside'? (y/n)$SPACE TODO: No tasks were deleted. @@ -71,10 +72,12 @@ TODO: No tasks were deleted. TODO: 3 of 3 tasks shown >>> printf x | todo.sh del 1 +=== 1 Delete '(B) smell the uppercase Roses +flowers @outside'? (y/n)$SPACE TODO: No tasks were deleted. >>> echo | todo.sh del 1 +=== 1 Delete '(B) smell the uppercase Roses +flowers @outside'? (y/n)$SPACE TODO: No tasks were deleted. diff --git a/tests/t1910-deduplicate.sh b/tests/t1910-deduplicate.sh index 3617807..6721786 100755 --- a/tests/t1910-deduplicate.sh +++ b/tests/t1910-deduplicate.sh @@ -32,6 +32,7 @@ EOF test_todo_session 'deduplicate without duplicates' <>> todo.sh deduplicate +=== 1 TODO: No duplicate tasks found EOF diff --git a/tests/t2120-shorthelp.sh b/tests/t2120-shorthelp.sh index f0123b7..82275db 100755 --- a/tests/t2120-shorthelp.sh +++ b/tests/t2120-shorthelp.sh @@ -47,7 +47,7 @@ echo 'export TODO_ACTIONS_DIR=$HOME/custom.actions' >> custom.cfg export TODOTXT_GLOBAL_CFG_FILE=global.cfg test_todo_session '-h and fatal error without config' <>> todo.sh -h | sed '/^ \\{0,2\\}[A-Z]/!d' +>>> todo.sh -h 2>&1 | sed '/^ \\{0,2\\}[A-Z]/!d' Usage: todo.sh [-fhpantvV] [-d todo_config] action [task_number] [task_description] Actions: Actions can be added and overridden using scripts in the actions @@ -58,7 +58,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' <>> todo.sh -h -d custom.cfg | sed '/^ \\{0,2\\}[A-Z]/!d' +>>> todo.sh -h -d custom.cfg 2>&1 | sed '/^ \\{0,2\\}[A-Z]/!d' Usage: todo.sh [-fhpantvV] [-d todo_config] action [task_number] [task_description] Actions: Actions can be added and overridden using scripts in the actions @@ -69,7 +69,7 @@ EOF # Config option processed; "Add-on Actions" is mentioned here. test_todo_session '-h output with preceding custom config' <>> todo.sh -d custom.cfg -h | sed '/^ \\{0,2\\}[A-Z]/!d' +>>> todo.sh -d custom.cfg -h 2>&1 | sed '/^ \\{0,2\\}[A-Z]/!d' Usage: todo.sh [-fhpantvV] [-d todo_config] action [task_number] [task_description] Actions: Actions can be added and overridden using scripts in the actions diff --git a/todo.sh b/todo.sh index 49d1655..50297f2 100755 --- a/todo.sh +++ b/todo.sh @@ -1170,7 +1170,7 @@ case $action in echo "TODO: $item deleted." fi else - echo "TODO: No tasks were deleted." + die "TODO: No tasks were deleted." fi else sed -i.bak \ @@ -1200,6 +1200,7 @@ case $action in # Split multiple depri's, if comma separated change to whitespace separated # Loop the 'depri' function for each item + status=0 for item in ${*//,/ }; do getTodo "$item" @@ -1211,9 +1212,11 @@ case $action in echo "TODO: $item deprioritized." fi else - echo "TODO: $item is not prioritized." + echo >&2 "TODO: $item is not prioritized." + status=1 fi done + exit $status ;; "do" | "done" ) @@ -1224,6 +1227,7 @@ case $action in # Split multiple do's, if comma separated change to whitespace separated # Loop the 'do' function for each item + status=0 for item in ${*//,/ }; do getTodo "$item" @@ -1239,15 +1243,17 @@ case $action in echo "TODO: $item marked as done." fi else - echo "TODO: $item is already marked done." + echo >&2 "TODO: $item is already marked done." + status=1 fi done if [ $TODOTXT_AUTO_ARCHIVE = 1 ]; then # Recursively invoke the script to allow overriding of the archive # action. - "$TODO_FULL_SH" archive + "$TODO_FULL_SH" archive || status=$? fi + exit $status ;; "help" ) @@ -1363,7 +1369,7 @@ case $action in echo "TODO: $item moved from '$src' to '$dest'." fi else - echo "TODO: No tasks moved." + die "TODO: No tasks moved." fi ;; @@ -1374,6 +1380,7 @@ case $action in "pri" | "p" ) shift + status=0 while [ "$#" -gt 0 ] ; do item=$1 newpri=$( printf "%s\n" "$2" | tr '[:lower:]' '[:upper:]' ) @@ -1405,10 +1412,12 @@ note: PRIORITY must be anywhere from A to Z." fi fi if [ "$oldpri" = "$newpri" ]; then - echo "TODO: $item already prioritized ($newpri)." + echo >&2 "TODO: $item already prioritized ($newpri)." + status=1 fi shift; shift done + exit $status ;; "replace" ) @@ -1476,7 +1485,7 @@ note: PRIORITY must be anywhere from A to Z." newTaskNum=$( sed -e '/./!d' "$TODO_FILE" | sed -n '$ =' ) deduplicateNum=$(( originalTaskNum - newTaskNum )) if [ $deduplicateNum -eq 0 ]; then - echo "TODO: No duplicate tasks found" + die "TODO: No duplicate tasks found" else echo "TODO: $deduplicateNum duplicate task(s) removed" fi