Skip to content
Merged
Changes from all commits
Commits
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
7 changes: 5 additions & 2 deletions install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -998,7 +998,10 @@ confirm() {
if [[ "$default" == "default_yes" ]]; then
$GUM confirm --default=yes "$text" < /dev/tty || rc=$?
else
$GUM confirm "$text" < /dev/tty || rc=$?
# --default=no is REQUIRED, not redundant: gum's --default is a bool that
# defaults to true, so omitting it preselects Yes and silently inverts every
# default_no prompt.
$GUM confirm --default=no "$text" < /dev/tty || rc=$?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Use false for Gum’s boolean default flag

With Gum v0.14.5, --default is a boolean whose explicit value is decoded through Kong’s strconv.ParseBool mapper; the Go documentation lists accepted false values as 0, f, and false, not no. Consequently --default=no is rejected before the TUI opens, and the surrounding || rc=$? silently converts that parse failure into a negative answer. During interactive installs, every default-no prompt—including Cognito, Telegram, AWS CLI updates, quota requests, and some continue/abort paths—therefore becomes impossible to accept; pass --default=false instead.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks — but this is a false positive, and gum's own error message is the disproof.

Gum does not use Kong's stock strconv.ParseBool mapper for this flag; it registers a custom bool mapper. Feeding it a value that mapper rejects makes it enumerate exactly what it accepts:

$ gum confirm --default=f q
gum: error: --default: bool value must be true, 1, yes, false, 0 or no but got "f"

So the accepted set is true, 1, yes, false, 0, noyes and no are valid, while t and f are the values that get rejected. That is the inverse of what strconv.ParseBool would do, which is what makes the ParseBool premise identifiable as the wrong model here.

Probed across the full range on gum v0.14.5 (linux/arm64), capturing stderr:

value parse error?
no none
false none
0 none
yes none
true none
1 none
f rejected
t rejected

Two further points against the finding as written:

  1. If yes/no were rejected, the pre-existing --default=yes on the affirmative branch — untouched by this PR and shipping for many releases — would have been silently broken the whole time, turning every default_yes prompt negative. That has not been observed.
  2. The claimed symptom is that default-no prompts become "impossible to accept". Under the actual mapper there is no parse failure, so rc reflects the user's real choice.

--default=false would also work and is equivalent; I'm keeping no because it reads symmetrically against the existing --default=yes on the adjacent branch. No code change.

Note the exit code alone cannot settle this: gum confirm returns 1 for every flag combination when it has no TTY, so a non-interactive exit-code probe is not valid evidence. The parse error on stderr is, because argument parsing happens before the TUI opens.

fi
[[ $rc -eq 130 ]] && { echo ""; cleanup_on_interrupt; }
return $rc
Expand All @@ -1014,7 +1017,7 @@ toggle() {
if [[ "$default" == "true" ]]; then
$GUM confirm --default=yes " $text" < /dev/tty || rc=$?
else
$GUM confirm " $text" < /dev/tty || rc=$?
$GUM confirm --default=no " $text" < /dev/tty || rc=$?
fi
[[ $rc -eq 0 ]] && printf -v "$var" '%s' "true" || printf -v "$var" '%s' "false"
}
Expand Down