Re: [PATCH v4] git-completion.bash: add support for path completion

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Junio C Hamano <gitster@xxxxxxxxx> writes:

> Marc Khouzam <marc.khouzam@xxxxxxxxxxxx> writes:
>
>> I've been playing with it but I'm not getting the expected 
>> behavior when I cd to a sub-directory.
>
> Thanks for testing.  Manlio?

Can you try the attached patch?

As I am not familiar with the completion machinery, take this with a
large grain of salt.  Here is my explanation of what is going on in
this "how about this" fixup:

 * Giving --git-dir from the command line (or GIT_DIR environment)
   without specifying GIT_WORK_TREE is to signal Git that you are at
   the top of the working tree.  "git ls-files" will then show the
   full tree even outside the real $cwd because you are lying to
   Git.

 * "git diff-index" could be told to show only the $cwd and its
   subdirectory with the "--relative" option, but it alone is not
   sufficient if you throw --git-dir at it; again, you end up lying
   that you are at the top.

 * As far as I can tell, there is no reason you would want to pass
   "--git-dir" to these invocations of ls-files and diff-index.  If
   the previous call to "__gitdir" could figure out where it is, the
   command should be able to figure it out the same way.
   
There seem to be millions of other existing "git --git-dir=$there"
in this script.  As I already said, I am not familiar with the
completion machinery, and I do not know what they are for in the
first place.  Perhaps people put them there for a reason, but I do
not know what that reason is.

I think the ones for "for-each-ref", "config" and "stash" should be
harmless.  They are commands that do not care about the working
tree.

There is one given to "ls-tree" used in __git_complete_revlist_file;
I do not know if this was intended, what it is for, and if that is
working as intended, though.

I've been CC'ing two people who touched this script rather heavily,
are expected to know the completion machinery, and should be able to
help polishing this topic and moving it forward.  Perhaps one of
them can shed light on this.

-- >8 --
Subject: completion: do not pass harmful "--git-dir=$there"

The recently added __git_index_files and __git_diff_index_files
helper functions invoke "ls-files" and "diff_index" while explicitly
passing "--git-dir=$there", to tell them that the invocation is done
at the top of the working tree, which may not be the case when the
user is in a subdirectory.  Remove the harmful use of this option,

Tell "diff-index" to show only the paths in the $cwd and show them
relative to the $cwd by passing "--relative". The "ls-files" does
not need this, as that is already its default mode of operation.

---
 contrib/completion/git-completion.bash | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index c8c6464..f4bd548 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -267,9 +267,9 @@ __git_index_files ()
 
 	if [ -d "$dir" ]; then
 		# NOTE: $1 is not quoted in order to support multiple options
-		git --git-dir="$dir" ls-files --exclude-standard $1 ${2+"$2"} 2>/dev/null |
-			__git_index_file_list_filter ${2+"$2"} |
-			uniq
+		git ls-files --exclude-standard $1 ${2+"$2"} 2>/dev/null |
+		__git_index_file_list_filter ${2+"$2"} |
+		uniq
 	fi
 }
 
@@ -284,9 +284,9 @@ __git_diff_index_files ()
 	local dir="$(__gitdir)"
 
 	if [ -d "$dir" ]; then
-		git --git-dir="$dir" diff-index --name-only "$1" 2>/dev/null |
-			__git_index_file_list_filter ${2+"$2"} |
-			uniq
+		git diff-index --name-only --relative "$1" 2>/dev/null |
+		__git_index_file_list_filter ${2+"$2"} |
+		uniq
 	fi
 }
 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]