diff mbox series

[v5,12/12] completion: fix 'git add' on paths under an untracked directory

Message ID 725adf0a9b89a5426579b73cc5a3a35999eac9a1.1585714667.git.gitgitgadget@gmail.com (mailing list archive)
State New, archived
Headers show
Series Avoid multiple recursive calls for same path in read_directory_recursive() | expand

Commit Message

Linus Arver via GitGitGadget April 1, 2020, 4:17 a.m. UTC 
From: Elijah Newren <newren@gmail.com>

As reported on the git mailing list, since git-2.25,
    git add untracked-dir/
has been tab completing to
    git add untracked-dir/./

The cause for this was that with commit b9670c1f5e (dir: fix checks on
common prefix directory, 2019-12-19),
    git ls-files -o --directory untracked-dir/
(or the equivalent `git -C untracked-dir ls-files -o --directory`) began
reporting
    untracked-dir/
instead of listing paths underneath that directory.  It may also be
worth noting that the real command in question was
    git -C untracked-dir ls-files -o --directory '*'
which is equivalent to
    git ls-files -o --directory 'untracked-dir/*'
which behaves the same for the purposes of this issue (the '*' can match
the empty string), but becomes relevant for the proposed fix.

At first, based on the report, I decided to try to view this as a
regression and tried to find a way to recover the old behavior without
breaking other stuff, or at least breaking as little as possible.
However, in the end, I couldn't figure out a way to do it that wouldn't
just cause lots more problems than it solved.  The old behavior was a
bug:
  * Although older git would avoid cleaning anything with `git clean -f
    .git`, it would wipe out everything under that direcotry with `git
    clean -f .git/`.  Despite the difference in command used, this is
    relevant because the exact same change that fixed clean changed the
    behavior of ls-files.
  * Older git would report different results based solely on presence or
    absence of a trailing slash for $SUBDIR in the command `git ls-files
    -o --directory $SUBDIR`.
  * Older git violated the documented behavior of not recursing into
    directories that matched the pathspec when --directory was
    specified.
  * And, after all, commit b9670c1f5e (dir: fix checks on common prefix
    directory, 2019-12-19) didn't overlook this issue; it explicitly
    stated that the behavior of the command was being changed to bring
    it inline with the docs.

(Also, if it helps, despite that commit being merged during the 2.25
series, this bug was not reported during the 2.25 cycle, nor even during
most of the 2.26 cycle -- it was reported a day before 2.26 was
released.  So the impact of the change is at least somewhat small.)

Instead of relying on a bug of ls-files in reporting the wrong content,
change the invocation of ls-files used by git-completion to make it grab
paths one depth deeper.  Do this by changing '$DIR/*' (match $DIR/ plus
0 or more characters) into '$DIR/?*' (match $DIR/ plus 1 or more
characters).  Note that the '?' character should not be added when
trying to complete a filename (e.g. 'git ls-files -o --directory
"merge.c?*"' would not correctly return "merge.c" when such a file
exists), so we have to make sure to add the '?' character only in cases
where the path specified so far is a directory.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 contrib/completion/git-completion.bash | 2 +-
 t/t9902-completion.sh                  | 5 +++++
 2 files changed, 6 insertions(+), 1 deletion(-)
diff mbox series

Patch

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index e4d9ff4a95c..1032b642297 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -504,7 +504,7 @@  __git_index_files ()
 {
 	local root="$2" match="$3"
 
-	__git_ls_files_helper "$root" "$1" "$match" |
+	__git_ls_files_helper "$root" "$1" "${match:-?}" |
 	awk -F / -v pfx="${2//\\/\\\\}" '{
 		paths[$1] = 1
 	}
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 93877ba9cd6..d9a6425671f 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -1581,6 +1581,11 @@  test_expect_success 'complete files' '
 	echo modify > modified &&
 	test_completion "git add " "modified" &&
 
+	mkdir -p some/deep &&
+	touch some/deep/path &&
+	test_completion "git add some/" "some/deep" &&
+	git clean -f some &&
+
 	touch untracked &&
 
 	: TODO .gitignore should not be here &&