From: Robert Estelle via GitGitGadget <hidden> Date: 2021-10-07 21:39:36
From: Robert Estelle <redacted>
In the basic `[`/`test` command, the string equality operator is a
single `=`. The `==` operator is only available in `[[`, which is a
bash-ism also supported by zsh.
This mix-up was causing the following completion error in zsh:
__git_ls_files_helper:7: = not found
(That message refers to the extraneous symbol in `==` ← `=`).
This updates that comparison to use the extended `[[ … ]]` conditional
for consistency with the other checks in this file.
Signed-off-by: Robert Estelle <redacted>
---
completion: Fix incorrect bash/zsh string equality check
This fixes an error in contrib/completion/git-completion.bash caused by
the incorrect use of == (vs. single =) inside a basic [/test command.
Double-equals == should only be used with the extended [[ comparison.
This was causing the following completion error in zsh:
> __git_ls_files_helper:7: = not found
That message refers to the extraneous = symbol in ==.
This updates that comparison to use the extended [[ … ]] conditional for
consistency with the other checks in this file.
Note that there may be some contributing cause to this error related to
emulation mode inheritance/stickiness, since it seems that the function
is intended to run with emulate ksh and that does not appear to be
happening properly. Nevertheless, fixing this comparison fixes this
particular error in a compatible way, and I have not observed any other
errors.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1096%2Frwe%2Ffix-completion-sh-eq-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1096/rwe/fix-completion-sh-eq-v1
Pull-Request: https://github.com/git/git/pull/1096
contrib/completion/git-completion.bash | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
@@ -515,7 +515,7 @@ __gitcomp_file ()# argument, and using the options specified in the second argument. __git_ls_files_helper(){-if["$2"=="--committable"];then+if[["$2"=="--committable"]];then__git-C"$1"-ccore.quotePath=falsediff-index\--name-only--relativeHEAD--"${3//\\/\\\\}*"else
From: Junio C Hamano <hidden> Date: 2021-10-08 20:50:39
"Robert Estelle via GitGitGadget" [off-list ref] writes:
This fixes an error in contrib/completion/git-completion.bash caused by
the incorrect use of == (vs. single =) inside a basic [/test command.
Double-equals == should only be used with the extended [[ comparison.
Curious.
Would it be equally a valid fix to use "=" instead of "==", or would
that change the meaning? This is a bash-only piece of code, so use
of [[ ... ]] is not technically incorrect, but if the basic [] works
well enough with "=", we should prefer that.
Thanks.
From: brian m. carlson <hidden> Date: 2021-10-08 20:58:13
On 2021-10-08 at 20:50:33, Junio C Hamano wrote:
"Robert Estelle via GitGitGadget" [off-list ref] writes:
quoted
This fixes an error in contrib/completion/git-completion.bash caused by
the incorrect use of == (vs. single =) inside a basic [/test command.
Double-equals == should only be used with the extended [[ comparison.
Curious.
Would it be equally a valid fix to use "=" instead of "==", or would
that change the meaning? This is a bash-only piece of code, so use
of [[ ... ]] is not technically incorrect, but if the basic [] works
well enough with "=", we should prefer that.
It's actually preferable in most cases to use [ and = rather than [[ and
==, because the former looks for strict equality and the latter looks
for pattern matching. If one is placing a glob pattern on the right
side, then [[ and == can be desirable, but otherwise it's better to
stick to the POSIX syntax.
--
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA
From: Robert Estelle <hidden> Date: 2021-10-08 22:05:45
On Fri, Oct 8, 2021 at 1:50 PM Junio C Hamano [off-list ref] wrote:
Would it be equally a valid fix to use "=" instead of "==", or would
that change the meaning? This is a bash-only piece of code, so use
of [[ ... ]] is not technically incorrect, but if the basic [] works
well enough with "=", we should prefer that.
Yes, `[` is preferable for portability and they'd behave the same in
this case. I consciously chose to use `[[` because that's what all the
other comparisons in that script use. (I think I noted that in the
commit message, maybe). I think there's value in consistency, and not
enough value of `[` over `[[` to justify changing all the other lines.
From: Robert Estelle <hidden> Date: 2021-10-08 22:17:32
On Fri, Oct 8, 2021 at 1:58 PM brian m. carlson
[off-list ref] wrote:
It's actually preferable in most cases to use [ and = rather than [[ and
==, because the former looks for strict equality and the latter looks
for pattern matching.
Yep, I agree with these style notes, but went with the prevalent style
to avoid confusion. And note that the args here are both quoted, and
so are treated as a literal comparison rather than globbed.
That said, whoever maintains that script, since this is a one-line
change, I'm not insistent about one way of addressing this or another
:)
From: Robert Estelle via GitGitGadget <hidden> Date: 2021-10-25 22:29:37
From: Robert Estelle <redacted>
In the basic `[`/`test` command, the string equality operator is a
single `=`. The `==` operator is only available in `[[`, which is a
bash-ism also supported by zsh.
This mix-up was causing the following completion error in zsh:
__git_ls_files_helper:7: = not found
(That message refers to the extraneous symbol in `==` ← `=`).
This updates that comparison to use a single `=` inside the
basic `[ … ]` test conditional.
Although this fix is inconsistent with the other comparisons in this
file, which use `[[ … == … ]]`, and the two expressions are functionally
identical in this context, that approach was rejected due to a
preference for `[`.
Signed-off-by: Robert Estelle <redacted>
---
completion: Fix incorrect bash/zsh string equality check
v2: This updates the comparison to remove the extraneous = symbol in ==,
and use the [ … ] conditional instead.
v1: (rejected) updated that comparison to use the extended [[ … ]]
conditional for consistency with the other checks in this file.
This fixes an error in contrib/completion/git-completion.bash caused by
the incorrect use of == (vs. single =) inside a basic [/test command.
Double-equals == should only be used with the extended [[ comparison.
This was causing the following completion error in zsh:
> __git_ls_files_helper:7: = not found
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1096%2Frwe%2Ffix-completion-sh-eq-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1096/rwe/fix-completion-sh-eq-v2
Pull-Request: https://github.com/git/git/pull/1096
Range-diff vs v1:
1: 6fd09347385 ! 1: eee166c8c99 completion: fix incorrect bash/zsh string equality check
@@ Commit message
(That message refers to the extraneous symbol in `==` ← `=`).
- This updates that comparison to use the extended `[[ … ]]` conditional
- for consistency with the other checks in this file.
+ This updates that comparison to use a single `=` inside the
+ basic `[ … ]` test conditional.
+
+ Although this fix is inconsistent with the other comparisons in this
+ file, which use `[[ … == … ]]`, and the two expressions are functionally
+ identical in this context, that approach was rejected due to a
+ preference for `[`.
Signed-off-by: Robert Estelle [off-list ref]
@@ contrib/completion/git-completion.bash: __gitcomp_file ()
__git_ls_files_helper ()
{
- if [ "$2" == "--committable" ]; then
-+ if [[ "$2" == "--committable" ]]; then
++ if [ "$2" = "--committable" ]; then
__git -C "$1" -c core.quotePath=false diff-index \
--name-only --relative HEAD -- "${3//\\/\\\\}*"
else
contrib/completion/git-completion.bash | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
@@ -515,7 +515,7 @@ __gitcomp_file ()# argument, and using the options specified in the second argument. __git_ls_files_helper(){-if["$2"=="--committable"];then+if["$2"="--committable"];then__git-C"$1"-ccore.quotePath=falsediff-index\--name-only--relativeHEAD--"${3//\\/\\\\}*"else