git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH] completion: fix array indexing error after reverse history search

From
SZEDER Gábor <szeder@ira.uka.de>
Date
May 10, 2011, 22:14 UTC
Message-ID
<20110510221430.GM1377@goldbirke>
In-Reply-To
<BANLkTi=nOUEp_J+2dkZZp=HvER-eAdG9eg@mail.gmail.com>

In v1.7.4-rc0~11^2~2 (bash: get --pretty=m<tab> completion to work with bash v4, 2010-12-02) we started to use _get_comp_words_by_ref() to access completion-related variables. This function is usually provided by the bash-completion package, but if it's not present when git-completion.bash is loaded, then we use our simplified re-implementation of that function. We also provide a bare bones _get_comp_words_by_ref() implementation that works with zsh.

Unfortunately (and interestingly!), since the recent commit da4902a (completion: remove unnecessary _get_comp_words_by_ref() invocations, 2011-04-28) the same bug can be triggered in all of these three implementations when doing reverse history search:

 - Hit ctrl-r to start reverse history search.
 - Search for a git command, e.g. by typing 'git '.
   The prompt will look like this: (reverse-i-search)`git ': git diff
 - Press TAB to attempt completion.
This will lead to an error like:
  $ bash: words: bad array subscript

The reason for this is that since commit da4902a we always tell _get_comp_words_by_ref() to set the $prev variable in _git(), which should contain the word preceeding the word containing the current cursor position. The value of this variable is assigned in all three _get_comp_words_by_ref() implementations with something like

  prev=${COMP_WORDS[COMP_CWORD-1]}

However, in the above bug-triggering input the cursor is considered to be at the very beginning of the command line, i.e. in the nullth word, so the array index in the above line ends up to be -1, hence the error. In this case the only sensible value for $prev would be an empty string, because there is no previous word on the command line.

The same applies to the _get_comp_words_by_ref() invocation in _gitk(), too.

This patch fixes both of our _get_comp_words_by_ref() implementations by assigning an empty string to $prev when there can't be any previous word. It also works around the bug in _get_comp_words_by_ref() from the bash-completion package by not asking for the $prev variable at all when there can't be any previous word. In the latter case we check the value of $COMP_CWORD; we usually refrain from using this variable directly in completion functions because of the word splitting changes introduced in bash v4 (see v1.7.4-rc0~11^2~2), but in this case, i.e. at the nullth word the different word splitting rules don't make a difference.

Noticed-by: Sverre Rabbelier <srabbelier@gmail.com>
Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
On Tue, May 10, 2011 at 10:13:11PM +0200, Sverre Rabbelier wrote:
Show 10 quoted lines
> This happens if I try use ctrl-shift-r (reverse-i-search) for the
> string `git commit -am "S`, resulting in the following:
> 
> (reverse-i-search)`git commit -am "S': git commit -am "Set new Melange
> version number to 2-0-20110501 in app.yaml.template."
> 
> If I then hit tab, I get:
> 
> $ bash: words: bad array subscriptversion number to 2-0-20110501 in
> app.yaml.template."

Nice catch, although I have no idea why anyone would attempt completion at that point.

Best, Gábor

 contrib/completion/git-completion.bash |   26 ++++++++++++++++++++++----
 1 files changed, 22 insertions(+), 4 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 00691fc..0b12c66 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -445,7 +445,11 @@ _get_comp_words_by_ref ()
 			cur=$cur_
 			;;
 		prev)
-			prev=${words_[$cword_-1]}
+			if [ "$cword_" = 0 ]; then
+				prev=""
+			else
+				prev=${words_[$cword_-1]}
+			fi
 			;;
 		words)
 			words=("${words_[@]}")
@@ -466,7 +470,11 @@ _get_comp_words_by_ref ()
 			cur=${COMP_WORDS[COMP_CWORD]}
 			;;
 		prev)
-			prev=${COMP_WORDS[COMP_CWORD-1]}
+			if [ "$COMP_CWORD" = 0 ]; then
+				prev=""
+			else
+				prev=${COMP_WORDS[COMP_CWORD-1]}
+			fi
 			;;
 		words)
 			words=("${COMP_WORDS[@]}")
@@ -2611,7 +2619,12 @@ _git ()
 	fi
 
 	local cur words cword prev
-	_get_comp_words_by_ref -n =: cur words cword prev
+	if [ "$COMP_CWORD" = 0 ]; then
+		_get_comp_words_by_ref -n =: cur words cword
+		prev=""
+	else
+		_get_comp_words_by_ref -n =: cur words cword prev
+	fi
 	while [ $c -lt $cword ]; do
 		i="${words[c]}"
 		case "$i" in
@@ -2662,7 +2675,12 @@ _gitk ()
 	fi
 
 	local cur words cword prev
-	_get_comp_words_by_ref -n =: cur words cword prev
+	if [ "$COMP_CWORD" = "0" ]; then
+		_get_comp_words_by_ref -n =: cur words cword
+		prev=""
+	else
+		_get_comp_words_by_ref -n =: cur words cword prev
+	fi
 
 	__git_has_doubledash && return
 
-- 
1.7.5.1.248.g2ba0c6
Previous: SZEDER GáborNext: Sverre Rabbelier
Message 11 of 12 in “[BUG] Autocompletion fails with "bash: words: bad array subscript"”
  1. Sverre RabbelierMay 10, 2011
  2. Jeff KingMay 10, 2011
  3. Jeff KingMay 10, 2011
  4. Sverre RabbelierMay 10, 2011
  5. Jeff KingMay 10, 2011
  6. Sverre RabbelierMay 10, 2011
  7. Jeff KingMay 10, 2011
  8. SZEDER GáborMay 10, 2011
  9. Jonathan NiederMay 10, 2011
  10. SZEDER GáborMay 11, 2011
  11. completion: fix array indexing error after reverse history searchSZEDER Gábor, May 10, 2011
  12. Sverre RabbelierMay 10, 2011

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.