Skip to content

Commit

Permalink
Refactoring: Use read -p MSG instead of doing echo -n MSG separately
Browse files Browse the repository at this point in the history
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.)
  • Loading branch information
inkarkat committed Jun 18, 2022
1 parent ea32af3 commit 4662651
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 19 deletions.
10 changes: 4 additions & 6 deletions tests/t1800-del.sh
Expand Up @@ -4,8 +4,6 @@ test_description='basic del functionality
'
. ./test-lib.sh

SPACE=' '

test_todo_session 'del usage' <<EOF
>>> todo.sh del B
usage: todo.sh del ITEM# [TERM]
Expand Down Expand Up @@ -60,7 +58,7 @@ test_todo_session 'del with confirmation' <<EOF
TODO: 3 of 3 tasks shown
>>> 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
Expand All @@ -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.
Expand Down
4 changes: 1 addition & 3 deletions tests/t1850-move.sh
Expand Up @@ -4,8 +4,6 @@ test_description='basic move functionality
'
. ./test-lib.sh

SPACE=' '

cat > todo.txt <<EOF
(B) smell the uppercase Roses +flowers @outside
(A) notice the sunflowers
Expand Down Expand Up @@ -42,7 +40,7 @@ x 2009-02-13 smell the coffee +wakeup
EOF
test_todo_session 'basic move with confirmation' <<EOF
>>> 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'.
Expand Down
15 changes: 5 additions & 10 deletions todo.sh
Expand Up @@ -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" ]
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 4662651

Please sign in to comment.