[PATCH] completion: hide dotfiles for selected path completion

[PATCH] completion: hide dotfiles for selected path completion

[Zakariyah Ali via GitGitGadget]

From: Zakariyah Ali <zakariyahali100@gmail.com>

Signed-off-by: Zakariyah Ali <zakariyahali100@gmail.com>
---
    completion: hide dotfiles for selected path completion
    
    The completion helper for index paths uses git ls-files rather than
    shell filename completion. As a result, leading-dot paths such as a
    tracked .gitignore were offered even when the user had not started the
    path with ..
    
    Hide leading-dot path components for git rm, git mv, and git ls-files
    when completing an empty path component. Explicit dot completion is
    still preserved, so git rm . can still complete .gitignore.
    
    This removes the existing TODO expectations in t/t9902-completion.sh and
    adds coverage for explicit dot completion.
    
    Validation:
    
     * git diff --check -- contrib/completion/git-completion.bash
       t/t9902-completion.sh
     * bash -n contrib/completion/git-completion.bash
     * ./t9902-completion.sh

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2311%2Falibaba0010%2Fcompletion-hide-dotfiles-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2311/alibaba0010/completion-hide-dotfiles-v1
Pull-Request: https://github.com/git/git/pull/2311

 contrib/completion/git-completion.bash | 36 +++++++++++++++++---------
 t/t9902-completion.sh                  | 10 ++-----
 2 files changed, 26 insertions(+), 20 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index a8e7c6ddbf..e8f8fab125 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -638,25 +638,33 @@ __git_ls_files_helper ()
 }
 
 
-# __git_index_files accepts 1 or 2 arguments:
+# __git_index_files accepts 1 to 4 arguments:
 # 1: Options to pass to ls-files (required).
 # 2: A directory path (optional).
 #    If provided, only files within the specified directory are listed.
 #    Sub directories are never recursed.  Path must have a trailing
 #    slash.
 # 3: List only paths matching this path component (optional).
+# 4: Hide paths whose first component starts with a dot if this is
+#    "hide-dotfiles" and the third argument is empty (optional).
 __git_index_files ()
 {
-	local root="$2" match="$3"
+	local root="$2" match="$3" hide_dotfiles="${4-}"
+	local hide_dotfiles_awk=0
+	if [ "$hide_dotfiles" = "hide-dotfiles" ] && [ -z "$match" ]; then
+		hide_dotfiles_awk=1
+	fi
 
 	__git_ls_files_helper "$root" "$1" "${match:-?}" |
-	awk -F / -v pfx="${2//\\/\\\\}" '{
+	awk -F / -v pfx="${2//\\/\\\\}" -v hide_dotfiles="$hide_dotfiles_awk" '{
 		paths[$1] = 1
 	}
 	END {
 		for (p in paths) {
 			if (substr(p, 1, 1) != "\"") {
 				# No special characters, easy!
+				if (hide_dotfiles == 1 && substr(p, 1, 1) == ".")
+					continue
 				print pfx p
 				continue
 			}
@@ -675,8 +683,10 @@ __git_index_files ()
 				# We have seen the same directory unquoted,
 				# skip it.
 				continue
-			else
-				print pfx p
+
+			if (hide_dotfiles == 1 && substr(p, 1, 1) == ".")
+				continue
+			print pfx p
 		}
 	}
 	function dequote(p,    bs_idx, out, esc, esc_idx, dec) {
@@ -721,13 +731,15 @@ __git_index_files ()
 	}'
 }
 
-# __git_complete_index_file requires 1 argument:
+# __git_complete_index_file accepts 1 or 2 arguments:
 # 1: the options to pass to ls-file
+# 2: Hide paths whose first component starts with a dot if this is
+#    "hide-dotfiles" and the current word is empty (optional).
 #
 # The exception is --committable, which finds the files appropriate commit.
 __git_complete_index_file ()
 {
-	local dequoted_word pfx="" cur_
+	local dequoted_word pfx="" cur_ hide_dotfiles="${2-}"
 
 	__git_dequote "$cur"
 
@@ -740,7 +752,7 @@ __git_complete_index_file ()
 		cur_="$dequoted_word"
 	esac
 
-	__gitcomp_file_direct "$(__git_index_files "$1" "$pfx" "$cur_")"
+	__gitcomp_file_direct "$(__git_index_files "$1" "$pfx" "$cur_" "$hide_dotfiles")"
 }
 
 # Lists branches from the local repository.
@@ -2164,7 +2176,7 @@ _git_ls_files ()
 
 	# XXX ignore options like --modified and always suggest all cached
 	# files.
-	__git_complete_index_file "--cached"
+	__git_complete_index_file "--cached" hide-dotfiles
 }
 
 _git_ls_remote ()
@@ -2397,9 +2409,9 @@ _git_mv ()
 	if [ $(__git_count_arguments "mv") -gt 0 ]; then
 		# We need to show both cached and untracked files (including
 		# empty directories) since this may not be the last argument.
-		__git_complete_index_file "--cached --others --directory"
+		__git_complete_index_file "--cached --others --directory" hide-dotfiles
 	else
-		__git_complete_index_file "--cached"
+		__git_complete_index_file "--cached" hide-dotfiles
 	fi
 }
 
@@ -3219,7 +3231,7 @@ _git_rm ()
 		;;
 	esac
 
-	__git_complete_index_file "--cached"
+	__git_complete_index_file "--cached" hide-dotfiles
 }
 
 _git_shortlog ()
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 28f61f08fb..02aaf71876 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -2811,17 +2811,15 @@ test_expect_success 'complete files' '
 
 	touch untracked &&
 
-	: TODO .gitignore should not be here &&
 	test_completion "git rm " <<-\EOF &&
-	.gitignore
 	modified
 	EOF
 
+	test_completion "git rm ." ".gitignore" &&
+
 	test_completion "git clean " "untracked" &&
 
-	: TODO .gitignore should not be here &&
 	test_completion "git mv " <<-\EOF &&
-	.gitignore
 	modified
 	EOF
 
@@ -2832,9 +2830,7 @@ test_expect_success 'complete files' '
 
 	mkdir untracked-dir &&
 
-	: TODO .gitignore should not be here &&
 	test_completion "git mv modified " <<-\EOF &&
-	.gitignore
 	dir
 	modified
 	untracked
@@ -2843,9 +2839,7 @@ test_expect_success 'complete files' '
 
 	test_completion "git commit " "modified" &&
 
-	: TODO .gitignore should not be here &&
 	test_completion "git ls-files " <<-\EOF &&
-	.gitignore
 	dir
 	modified
 	EOF

base-commit: 9b7fa37559a1b95ee32e32858b0d038b4cf583e5
-- 
gitgitgadget

Re: [PATCH] completion: hide dotfiles for selected path completion

[Junio C Hamano]

"Zakariyah Ali via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Zakariyah Ali <zakariyahali100@gmail.com>
>
> Signed-off-by: Zakariyah Ali <zakariyahali100@gmail.com>
> ---
>     completion: hide dotfiles for selected path completion
>     
>     The completion helper for index paths uses git ls-files rather than
>     shell filename completion. As a result, leading-dot paths such as a
>     tracked .gitignore were offered even when the user had not started the
>     path with ..

Writing 'path with ".".' would have been easieer to grok.

>     Hide leading-dot path components for git rm, git mv, and git ls-files
>     when completing an empty path component. Explicit dot completion is
>     still preserved, so git rm . can still complete .gitignore.

I am not sure why this is a good idea.  If we said "git rm g<TAB>
and offered ".gitignore" as a candidate, it may be annoying, but
tracked (or untracked for that matter) ".gitignore" and "gitfoo"
should be treated the same way by "git rm <TAB>" no?

>     This removes the existing TODO expectations in t/t9902-completion.sh and
>     adds coverage for explicit dot completion.

In any case, all of the above should be in the proposed log message,
not below the three-dash line.

[PATCH v2] completion: hide dotfiles for selected path completion

[Zakariyah Ali via GitGitGadget]

From: Zakariyah Ali <zakariyahali100@gmail.com>

The completion helper for index paths uses git ls-files rather than
shell filename completion. As a result, leading-dot paths such as a
tracked .gitignore were offered even when the user had not started the
path with ".".

Hide leading-dot path components for git rm, git mv, and git ls-files
when completing an empty path component. Explicit dot completion is
still preserved, so git rm . can still complete .gitignore.

This matches standard shell filename completion behavior, where dotfiles
are hidden by default unless the user starts their input with a dot.
This also resolves four TODO comments in t/9902-completion.sh which
have been present since 2013 (commit ddf07bddef9a, "completion: add file
completion tests", 2013-04-27), expecting that .gitignore would not be
shown when completing on an empty path component.

Signed-off-by: Zakariyah Ali <zakariyahali100@gmail.com>
---
    completion: hide dotfiles for selected path completion
    
    The completion helper for index paths uses git ls-files rather than
    shell filename completion. As a result, leading-dot paths such as a
    tracked .gitignore were offered even when the user had not started the
    path with ..
    
    Hide leading-dot path components for git rm, git mv, and git ls-files
    when completing an empty path component. Explicit dot completion is
    still preserved, so git rm . can still complete .gitignore.
    
    This removes the existing TODO expectations in t/t9902-completion.sh and
    adds coverage for explicit dot completion.
    
    Validation:
    
     * git diff --check -- contrib/completion/git-completion.bash
       t/t9902-completion.sh
     * bash -n contrib/completion/git-completion.bash
     * ./t9902-completion.sh

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2311%2Falibaba0010%2Fcompletion-hide-dotfiles-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2311/alibaba0010/completion-hide-dotfiles-v2
Pull-Request: https://github.com/git/git/pull/2311

Range-diff vs v1:

 1:  5ccb408fd5 ! 1:  056e239e06 completion: hide dotfiles for selected path completion
     @@ Metadata
       ## Commit message ##
          completion: hide dotfiles for selected path completion
      
     +    The completion helper for index paths uses git ls-files rather than
     +    shell filename completion. As a result, leading-dot paths such as a
     +    tracked .gitignore were offered even when the user had not started the
     +    path with ".".
     +
     +    Hide leading-dot path components for git rm, git mv, and git ls-files
     +    when completing an empty path component. Explicit dot completion is
     +    still preserved, so git rm . can still complete .gitignore.
     +
     +    This matches standard shell filename completion behavior, where dotfiles
     +    are hidden by default unless the user starts their input with a dot.
     +    This also resolves four TODO comments in t/9902-completion.sh which
     +    have been present since 2013 (commit ddf07bddef9a, "completion: add file
     +    completion tests", 2013-04-27), expecting that .gitignore would not be
     +    shown when completing on an empty path component.
     +
          Signed-off-by: Zakariyah Ali <zakariyahali100@gmail.com>
      
       ## contrib/completion/git-completion.bash ##


 contrib/completion/git-completion.bash | 36 +++++++++++++++++---------
 t/t9902-completion.sh                  | 10 ++-----
 2 files changed, 26 insertions(+), 20 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index a8e7c6ddbf..e8f8fab125 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -638,25 +638,33 @@ __git_ls_files_helper ()
 }
 
 
-# __git_index_files accepts 1 or 2 arguments:
+# __git_index_files accepts 1 to 4 arguments:
 # 1: Options to pass to ls-files (required).
 # 2: A directory path (optional).
 #    If provided, only files within the specified directory are listed.
 #    Sub directories are never recursed.  Path must have a trailing
 #    slash.
 # 3: List only paths matching this path component (optional).
+# 4: Hide paths whose first component starts with a dot if this is
+#    "hide-dotfiles" and the third argument is empty (optional).
 __git_index_files ()
 {
-	local root="$2" match="$3"
+	local root="$2" match="$3" hide_dotfiles="${4-}"
+	local hide_dotfiles_awk=0
+	if [ "$hide_dotfiles" = "hide-dotfiles" ] && [ -z "$match" ]; then
+		hide_dotfiles_awk=1
+	fi
 
 	__git_ls_files_helper "$root" "$1" "${match:-?}" |
-	awk -F / -v pfx="${2//\\/\\\\}" '{
+	awk -F / -v pfx="${2//\\/\\\\}" -v hide_dotfiles="$hide_dotfiles_awk" '{
 		paths[$1] = 1
 	}
 	END {
 		for (p in paths) {
 			if (substr(p, 1, 1) != "\"") {
 				# No special characters, easy!
+				if (hide_dotfiles == 1 && substr(p, 1, 1) == ".")
+					continue
 				print pfx p
 				continue
 			}
@@ -675,8 +683,10 @@ __git_index_files ()
 				# We have seen the same directory unquoted,
 				# skip it.
 				continue
-			else
-				print pfx p
+
+			if (hide_dotfiles == 1 && substr(p, 1, 1) == ".")
+				continue
+			print pfx p
 		}
 	}
 	function dequote(p,    bs_idx, out, esc, esc_idx, dec) {
@@ -721,13 +731,15 @@ __git_index_files ()
 	}'
 }
 
-# __git_complete_index_file requires 1 argument:
+# __git_complete_index_file accepts 1 or 2 arguments:
 # 1: the options to pass to ls-file
+# 2: Hide paths whose first component starts with a dot if this is
+#    "hide-dotfiles" and the current word is empty (optional).
 #
 # The exception is --committable, which finds the files appropriate commit.
 __git_complete_index_file ()
 {
-	local dequoted_word pfx="" cur_
+	local dequoted_word pfx="" cur_ hide_dotfiles="${2-}"
 
 	__git_dequote "$cur"
 
@@ -740,7 +752,7 @@ __git_complete_index_file ()
 		cur_="$dequoted_word"
 	esac
 
-	__gitcomp_file_direct "$(__git_index_files "$1" "$pfx" "$cur_")"
+	__gitcomp_file_direct "$(__git_index_files "$1" "$pfx" "$cur_" "$hide_dotfiles")"
 }
 
 # Lists branches from the local repository.
@@ -2164,7 +2176,7 @@ _git_ls_files ()
 
 	# XXX ignore options like --modified and always suggest all cached
 	# files.
-	__git_complete_index_file "--cached"
+	__git_complete_index_file "--cached" hide-dotfiles
 }
 
 _git_ls_remote ()
@@ -2397,9 +2409,9 @@ _git_mv ()
 	if [ $(__git_count_arguments "mv") -gt 0 ]; then
 		# We need to show both cached and untracked files (including
 		# empty directories) since this may not be the last argument.
-		__git_complete_index_file "--cached --others --directory"
+		__git_complete_index_file "--cached --others --directory" hide-dotfiles
 	else
-		__git_complete_index_file "--cached"
+		__git_complete_index_file "--cached" hide-dotfiles
 	fi
 }
 
@@ -3219,7 +3231,7 @@ _git_rm ()
 		;;
 	esac
 
-	__git_complete_index_file "--cached"
+	__git_complete_index_file "--cached" hide-dotfiles
 }
 
 _git_shortlog ()
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 28f61f08fb..02aaf71876 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -2811,17 +2811,15 @@ test_expect_success 'complete files' '
 
 	touch untracked &&
 
-	: TODO .gitignore should not be here &&
 	test_completion "git rm " <<-\EOF &&
-	.gitignore
 	modified
 	EOF
 
+	test_completion "git rm ." ".gitignore" &&
+
 	test_completion "git clean " "untracked" &&
 
-	: TODO .gitignore should not be here &&
 	test_completion "git mv " <<-\EOF &&
-	.gitignore
 	modified
 	EOF
 
@@ -2832,9 +2830,7 @@ test_expect_success 'complete files' '
 
 	mkdir untracked-dir &&
 
-	: TODO .gitignore should not be here &&
 	test_completion "git mv modified " <<-\EOF &&
-	.gitignore
 	dir
 	modified
 	untracked
@@ -2843,9 +2839,7 @@ test_expect_success 'complete files' '
 
 	test_completion "git commit " "modified" &&
 
-	: TODO .gitignore should not be here &&
 	test_completion "git ls-files " <<-\EOF &&
-	.gitignore
 	dir
 	modified
 	EOF

base-commit: 9b7fa37559a1b95ee32e32858b0d038b4cf583e5
-- 
gitgitgadget

Re: [PATCH v2] completion: hide dotfiles for selected path completion

[Junio C Hamano]

"Zakariyah Ali via GitGitGadget" <gitgitgadget@gmail.com> writes:

> This matches standard shell filename completion behavior, where dotfiles
> are hidden by default unless the user starts their input with a dot.

OK, with this rationale added, I no longer have problem with the
proposed new behaviour.

As I'm not going to give a serious review  on the patch body itself,
I would really appreciate somebody more knowledgeable on the
existing bach completion code than I am to take a look.

Thanks.