From 1a5600c79cd7c586ef4d98abfd010e016dbff7be Mon Sep 17 00:00:00 2001 From: Pegasust Date: Thu, 16 Jun 2022 16:28:05 -0700 Subject: [PATCH 1/3] test listaddons: Make pass on Cygwin 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. --- tests/t8010-listaddons.sh | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/t8010-listaddons.sh b/tests/t8010-listaddons.sh index b4e89aa..cf212e8 100755 --- a/tests/t8010-listaddons.sh +++ b/tests/t8010-listaddons.sh @@ -30,9 +30,11 @@ EOF chmod -x .todo.actions.d/foo # On Cygwin, clearing the executable flag may have no effect, as the Windows ACL -# may still grant execution rights. In this case, we skip the test. +# may still grant execution rights. In this case, we skip the test, and remove +# the (still valid) custom action so that it doesn't break following tests. if [ -x .todo.actions.d/foo ]; then SKIP_TESTS="${SKIP_TESTS}${SKIP_TESTS+ }t8010.4" + rm .todo.actions.d/foo fi test_todo_session 'nonexecutable action' <>> todo.sh listaddons @@ -69,9 +71,11 @@ EOF # nthorne: shamelessly stolen from above.. chmod -x .todo.actions.d/norris/norris # On Cygwin, clearing the executable flag may have no effect, as the Windows ACL -# may still grant execution rights. In this case, we skip the test. +# may still grant execution rights. In this case, we skip the test, and remove +# the (still valid) custom action so that it doesn't break following tests. if [ -x .todo.actions.d/norris/norris ]; then SKIP_TESTS="${SKIP_TESTS}${SKIP_TESTS+ }t8010.8" + rm .todo.actions.d/norris/norris fi test_todo_session 'nonexecutable action in subfolder' <>> todo.sh listaddons From bab2af950186380d6106702768a5a7cce7d84de0 Mon Sep 17 00:00:00 2001 From: Ingo Karkat Date: Sat, 18 Jun 2022 10:55:04 +0200 Subject: [PATCH 2/3] Tests: Refactoring: Extract invalidate_action() into actions-test-lib We don't "shamelessly steal" code, we refactoring it ;-) --- tests/actions-test-lib.sh | 16 ++++++++++++++++ tests/t8010-listaddons.sh | 19 ++----------------- 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/tests/actions-test-lib.sh b/tests/actions-test-lib.sh index 51d4eef..daf18a9 100644 --- a/tests/actions-test-lib.sh +++ b/tests/actions-test-lib.sh @@ -34,3 +34,19 @@ echo "custom action $1 in folder $1" EOF chmod +x ".todo.actions.d/$1/$1" } + +invalidate_action() +{ + local customActionFilespec="${1:?}"; shift + local testName="${1:?}"; shift + + chmod -x "$customActionFilespec" + # On Cygwin, clearing the executable flag may have no effect, as the Windows + # ACL may still grant execution rights. In this case, we skip the test, and + # remove the (still valid) custom action so that it doesn't break following + # tests. + if [ -x "$customActionFilespec" ]; then + SKIP_TESTS="${SKIP_TESTS}${SKIP_TESTS+ }${testName}" + rm -- "$customActionFilespec" + fi +} diff --git a/tests/t8010-listaddons.sh b/tests/t8010-listaddons.sh index cf212e8..ee0495a 100755 --- a/tests/t8010-listaddons.sh +++ b/tests/t8010-listaddons.sh @@ -28,14 +28,7 @@ ls quux EOF -chmod -x .todo.actions.d/foo -# On Cygwin, clearing the executable flag may have no effect, as the Windows ACL -# may still grant execution rights. In this case, we skip the test, and remove -# the (still valid) custom action so that it doesn't break following tests. -if [ -x .todo.actions.d/foo ]; then - SKIP_TESTS="${SKIP_TESTS}${SKIP_TESTS+ }t8010.4" - rm .todo.actions.d/foo -fi +invalidate_action .todo.actions.d/foo t8010.4 test_todo_session 'nonexecutable action' <>> todo.sh listaddons bar @@ -68,15 +61,7 @@ norris quux EOF -# nthorne: shamelessly stolen from above.. -chmod -x .todo.actions.d/norris/norris -# On Cygwin, clearing the executable flag may have no effect, as the Windows ACL -# may still grant execution rights. In this case, we skip the test, and remove -# the (still valid) custom action so that it doesn't break following tests. -if [ -x .todo.actions.d/norris/norris ]; then - SKIP_TESTS="${SKIP_TESTS}${SKIP_TESTS+ }t8010.8" - rm .todo.actions.d/norris/norris -fi +invalidate_action .todo.actions.d/norris/norris t8010.8 test_todo_session 'nonexecutable action in subfolder' <>> todo.sh listaddons bar From 466265175bdb119f1dbe1ecb5dd78af37d4528f3 Mon Sep 17 00:00:00 2001 From: Ingo Karkat Date: Fri, 17 Jun 2022 23:39:42 +0200 Subject: [PATCH 3/3] Refactoring: Use read -p MSG instead of doing echo -n MSG separately 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.) --- tests/t1800-del.sh | 10 ++++------ tests/t1850-move.sh | 4 +--- todo.sh | 15 +++++---------- 3 files changed, 10 insertions(+), 19 deletions(-) diff --git a/tests/t1800-del.sh b/tests/t1800-del.sh index 6d96e89..2529a93 100755 --- a/tests/t1800-del.sh +++ b/tests/t1800-del.sh @@ -4,8 +4,6 @@ test_description='basic del functionality ' . ./test-lib.sh -SPACE=' ' - test_todo_session 'del usage' <>> todo.sh del B usage: todo.sh del ITEM# [TERM] @@ -60,7 +58,7 @@ test_todo_session 'del with confirmation' <>> printf n | todo.sh del 1 -Delete '(B) smell the uppercase Roses +flowers @outside'? (y/n)$SPACE +\\ TODO: No tasks were deleted. >>> todo.sh -p list @@ -71,15 +69,15 @@ TODO: No tasks were deleted. TODO: 3 of 3 tasks shown >>> printf x | todo.sh del 1 -Delete '(B) smell the uppercase Roses +flowers @outside'? (y/n)$SPACE +\\ TODO: No tasks were deleted. >>> echo | todo.sh del 1 -Delete '(B) smell the uppercase Roses +flowers @outside'? (y/n)$SPACE +\\ TODO: No tasks were deleted. >>> printf y | todo.sh del 1 -Delete '(B) smell the uppercase Roses +flowers @outside'? (y/n)$SPACE +\\ 1 (B) smell the uppercase Roses +flowers @outside TODO: 1 deleted. diff --git a/tests/t1850-move.sh b/tests/t1850-move.sh index 9fbdc0b..2a9e6ea 100755 --- a/tests/t1850-move.sh +++ b/tests/t1850-move.sh @@ -4,8 +4,6 @@ test_description='basic move functionality ' . ./test-lib.sh -SPACE=' ' - cat > todo.txt <>> printf y | todo.sh move 1 done.txt 2>&1 | sed -e "s#'[^']\{1,\}/\([^/']\{1,\}\)'#'\1'#g" -e 's#from .\{1,\}/\([^/]\{1,\}\) to .\{1,\}/\([^/]\{1,\}\)?#from \1 to \2?#g' -Move '(B) smell the uppercase Roses +flowers @outside' from todo.txt to done.txt? (y/n)$SPACE +\\ 1 (B) smell the uppercase Roses +flowers @outside TODO: 1 moved from 'todo.txt' to 'done.txt'. diff --git a/todo.sh b/todo.sh index fb8d4bc..c3d4675 100755 --- a/todo.sh +++ b/todo.sh @@ -364,12 +364,11 @@ confirm() { [ $TODOTXT_FORCE = 0 ] || return 0 - printf %s "${1:?}? (y/n) " local readArgs=(-e -r) [ -n "${BASH_VERSINFO:-}" ] && [ \( ${BASH_VERSINFO[0]} -eq 4 -a ${BASH_VERSINFO[1]} -ge 1 \) -o ${BASH_VERSINFO[0]} -gt 4 ] && readArgs+=(-N 1) # Bash 4.1+ supports -N nchars local answer - read "${readArgs[@]}" answer + read -p "${1:?}? (y/n) " "${readArgs[@]}" answer echo [ "$answer" = "y" ] } @@ -451,8 +450,7 @@ replaceOrPrepend() getTodo "$item" if [[ -z "$1" && $TODOTXT_FORCE = 0 ]]; then - echo -n "$querytext" - read -r -i "$todo" -e input + read -p "$querytext" -r -i "$todo" -e input else input=$* fi @@ -1065,8 +1063,7 @@ fi case $action in "add" | "a") if [[ -z "$2" && $TODOTXT_FORCE = 0 ]]; then - echo -n "Add: " - read -e -r input + read -p "Add: " -e -r input else [ -z "$2" ] && die "usage: $TODO_SH add \"TODO ITEM\"" shift @@ -1077,8 +1074,7 @@ case $action in "addm") if [[ -z "$2" && $TODOTXT_FORCE = 0 ]]; then - echo -n "Add: " - read -e -r input + read -p "Add: " -e -r input else [ -z "$2" ] && die "usage: $TODO_SH addm \"TODO ITEM\"" shift @@ -1118,8 +1114,7 @@ case $action in getTodo "$item" if [[ -z "$1" && $TODOTXT_FORCE = 0 ]]; then - echo -n "Append: " - read -e -r input + read -p "Append: " -e -r input else input=$* fi