# [PATCH v2 0/9] completion: general cleanups 15 messages from 2013-04-27 to 2013-04-28. Participants: Felipe Contreras, Junio C Hamano, Eric Sunshine. Thread: https://gitlist.dev/t/33654 ## Felipe Contreras, 2013-04-27 20:09 Subject: [PATCH v2 0/9] completion: general cleanups Message-ID: <1367093407-8898-1-git-send-email-felipe.contreras@gmail.com> URL: https://gitlist.dev/e/1367093407-8898-1-git-send-email-felipe.contreras%40gmail.com ``` Hi, In the previous series I didn't notice -o filenames actually did something useful, so in this series it's still used, however, for versions older than 4, we use a hack to enable it. This allows us to get rid of a lot of compat code. I also got rid of some wrappers, and fixed a regression: spaces were not added at the end of the completion. The result is that now we have the same completion for bash < 4, bash >=4, and zsh. Although we already had that for the last two, now the code is simpler. The only difference is that bash < 4 doesn't add a trailing space, like before fea16b4 (git-completion.bash: add support for path completion). Also, add the missing tests. Cheers. Felipe Contreras (9): completion: add file completion tests completion: document tilde expansion failure in tests completion; remove unuseful comments completion: use __gitcompadd for __gitcomp_file completion: refactor diff_index wrappers completion: refactor __git_complete_index_file() completion: add hack to enable file mode in bash < 4 completion: add space after completed filename completion: remove __git_index_file_list_filter() contrib/completion/git-completion.bash | 145 +++++++-------------------------- t/t9902-completion.sh | 77 +++++++++++++++++ 2 files changed, 107 insertions(+), 115 deletions(-) -- 1.8.2.1.1031.g2ee5873 ``` ## Felipe Contreras, 2013-04-27 20:09 Subject: [PATCH v2 1/9] completion: add file completion tests Message-ID: <1367093407-8898-2-git-send-email-felipe.contreras@gmail.com> URL: https://gitlist.dev/e/1367093407-8898-2-git-send-email-felipe.contreras%40gmail.com In-Reply-To: <1367093407-8898-1-git-send-email-felipe.contreras@gmail.com> ``` The commit fea16b4 (git-completion.bash: add support for path completion) introduced quite a few changes, without the usual tests. Signed-off-by: Felipe Contreras --- t/t9902-completion.sh | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh index 6d9d141..385e1e4 100755 --- a/t/t9902-completion.sh +++ b/t/t9902-completion.sh @@ -347,4 +347,72 @@ test_expect_success 'send-email' ' test_completion "git send-email ma" "master " ' +test_expect_success 'complete files' ' + git init tmp && cd tmp && + test_when_finished "cd .. && rm -rf tmp" && + + echo "expected" > .gitignore && + echo "out" >> .gitignore && + + git add .gitignore && + test_completion "git commit " ".gitignore" && + + git commit -m ignore && + + touch new && + test_completion "git add " "new" && + + git add new && + git commit -a -m new && + test_completion "git add " "" && + + git mv new modified && + echo modify > modified && + test_completion "git add " "modified" && + + touch untracked && + + : TODO .gitignore should not be here && + test_completion "git rm " <<-\EOF && + .gitignore + modified + EOF + + test_completion "git clean " "untracked" && + + : TODO .gitignore should not be here && + test_completion "git mv " <<-\EOF && + .gitignore + modified + EOF + + mkdir dir && + touch dir/file-in-dir && + git add dir/file-in-dir && + git commit -m dir && + + mkdir untracked-dir && + + : TODO .gitignore should not be here && + test_completion "git mv modified " <<-\EOF && + .gitignore + dir + modified + untracked + untracked-dir + EOF + + test_completion "git commit " "modified" && + + : TODO .gitignore should not be here && + test_completion "git ls-files " <<-\EOF + .gitignore + dir + modified + EOF + + touch momified && + test_completion "git add mom" "momified" +' + test_done -- 1.8.2.1.1031.g2ee5873 ``` ## Felipe Contreras, 2013-04-27 20:10 Subject: [PATCH v2 2/9] completion: document tilde expansion failure in tests Message-ID: <1367093407-8898-3-git-send-email-felipe.contreras@gmail.com> URL: https://gitlist.dev/e/1367093407-8898-3-git-send-email-felipe.contreras%40gmail.com In-Reply-To: <1367093407-8898-1-git-send-email-felipe.contreras@gmail.com> ``` Signed-off-by: Felipe Contreras --- t/t9902-completion.sh | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh index 385e1e4..81a1657 100755 --- a/t/t9902-completion.sh +++ b/t/t9902-completion.sh @@ -415,4 +415,13 @@ test_expect_success 'complete files' ' test_completion "git add mom" "momified" ' +test_expect_failure 'complete with tilde expansion' ' + git init tmp && cd tmp && + test_when_finished "cd .. && rm -rf tmp" && + + touch ~/tmp/file && + + test_completion "git add ~/tmp/" "~/tmp/file" +' + test_done -- 1.8.2.1.1031.g2ee5873 ``` ## Felipe Contreras, 2013-04-27 20:10 Subject: [PATCH v2 3/9] completion; remove unuseful comments Message-ID: <1367093407-8898-4-git-send-email-felipe.contreras@gmail.com> URL: https://gitlist.dev/e/1367093407-8898-4-git-send-email-felipe.contreras%40gmail.com In-Reply-To: <1367093407-8898-1-git-send-email-felipe.contreras@gmail.com> ``` The only caller, __git_complete_index_file() doesn't specify any limits to the options for 'git ls-files', neither should this function. Signed-off-by: Felipe Contreras --- contrib/completion/git-completion.bash | 2 -- 1 file changed, 2 deletions(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index bc3fc9e..f7b0f3c 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -323,8 +323,6 @@ __git_diff_index_helper () # __git_index_files accepts 1 or 2 arguments: # 1: Options to pass to ls-files (required). -# Supported options are --cached, --modified, --deleted, --others, -# and --directory. # 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 -- 1.8.2.1.1031.g2ee5873 ``` ## Felipe Contreras, 2013-04-27 20:10 Subject: [PATCH v2 4/9] completion: use __gitcompadd for __gitcomp_file Message-ID: <1367093407-8898-5-git-send-email-felipe.contreras@gmail.com> URL: https://gitlist.dev/e/1367093407-8898-5-git-send-email-felipe.contreras%40gmail.com In-Reply-To: <1367093407-8898-1-git-send-email-felipe.contreras@gmail.com> ``` Like the rest of the script does; let's not access COMPREPLY directly. Signed-off-by: Felipe Contreras --- contrib/completion/git-completion.bash | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index f7b0f3c..7f1ebe4 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -252,7 +252,7 @@ __gitcomp_file () # since tilde expansion is not applied. # This means that COMPREPLY will be empty and Bash default # completion will be used. - COMPREPLY=($(compgen -P "${2-}" -W "$1" -- "${3-$cur}")) + __gitcompadd "$1" "${2-}" "${3-$cur}" "" # Tell Bash that compspec generates filenames. compopt -o filenames 2>/dev/null -- 1.8.2.1.1031.g2ee5873 ``` ## Felipe Contreras, 2013-04-27 20:10 Subject: [PATCH v2 5/9] completion: refactor diff_index wrappers Message-ID: <1367093407-8898-6-git-send-email-felipe.contreras@gmail.com> URL: https://gitlist.dev/e/1367093407-8898-6-git-send-email-felipe.contreras%40gmail.com In-Reply-To: <1367093407-8898-1-git-send-email-felipe.contreras@gmail.com> ``` At the end of the day what we really need is to find out the files that have been staged, or modified, because those files are the ones that make sense to pass as arguments to 'git commit'. We need diff-index to find those out, since 'git ls-files' doesn't do that. But we don't need wrappers and wrappers basically identical to the ones used for 'git ls-files', when we can pretend it receives a --committable option that would return what we need. That way, we can remove a bunch of code without any functional changes. Signed-off-by: Felipe Contreras --- contrib/completion/git-completion.bash | 71 ++++++++-------------------------- 1 file changed, 16 insertions(+), 55 deletions(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 7f1ebe4..2561265 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -297,30 +297,25 @@ __git_index_file_list_filter () __git_index_file_list_filter_bash } -# Execute git ls-files, returning paths relative to the directory -# specified in the first argument, and using the options specified in -# the second argument. +# Execute 'git ls-files', unless the --committable option is specified, in +# which case it runs 'git diff-index' to find out the files that can be +# committed. It return paths relative to the directory specified in the first +# argument, and using the options specified in the second argument. __git_ls_files_helper () { ( test -n "${CDPATH+set}" && unset CDPATH - # NOTE: $2 is not quoted in order to support multiple options - cd "$1" && git ls-files --exclude-standard $2 + cd "$1" + if [ "$2" == "--committable" ]; then + git diff-index --name-only --relative HEAD + else + # NOTE: $2 is not quoted in order to support multiple options + git ls-files --exclude-standard $2 + fi ) 2>/dev/null } -# Execute git diff-index, returning paths relative to the directory -# specified in the first argument, and using the tree object id -# specified in the second argument. -__git_diff_index_helper () -{ - ( - test -n "${CDPATH+set}" && unset CDPATH - cd "$1" && git diff-index --name-only --relative "$2" - ) 2>/dev/null -} - # __git_index_files accepts 1 or 2 arguments: # 1: Options to pass to ls-files (required). # 2: A directory path (optional). @@ -337,22 +332,6 @@ __git_index_files () fi } -# __git_diff_index_files accepts 1 or 2 arguments: -# 1) The id of a tree object. -# 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. -__git_diff_index_files () -{ - local dir="$(__gitdir)" root="${2-.}" - - if [ -d "$dir" ]; then - __git_diff_index_helper "$root" "$1" | __git_index_file_list_filter | - sort | uniq - fi -} - __git_heads () { local dir="$(__gitdir)" @@ -550,8 +529,10 @@ __git_complete_revlist_file () } -# __git_complete_index_file requires 1 argument: the options to pass to -# ls-file +# __git_complete_index_file requires 1 argument: +# 1: the options to pass to ls-file +# +# The exception is --committable, which finds the files appropriate commit. __git_complete_index_file () { local pfx cur_="$cur" @@ -570,26 +551,6 @@ __git_complete_index_file () esac } -# __git_complete_diff_index_file requires 1 argument: the id of a tree -# object -__git_complete_diff_index_file () -{ - local pfx cur_="$cur" - - case "$cur_" in - ?*/*) - pfx="${cur_%/*}" - cur_="${cur_##*/}" - pfx="${pfx}/" - - __gitcomp_file "$(__git_diff_index_files "$1" "$pfx")" "$pfx" "$cur_" - ;; - *) - __gitcomp_file "$(__git_diff_index_files "$1")" "" "$cur_" - ;; - esac -} - __git_complete_file () { __git_complete_revlist_file @@ -1211,7 +1172,7 @@ _git_commit () esac if git rev-parse --verify --quiet HEAD >/dev/null; then - __git_complete_diff_index_file "HEAD" + __git_complete_index_file "--committable" else # This is the first commit __git_complete_index_file "--cached" -- 1.8.2.1.1031.g2ee5873 ``` ## Felipe Contreras, 2013-04-27 20:10 Subject: [PATCH v2 6/9] completion: refactor __git_complete_index_file() Message-ID: <1367093407-8898-7-git-send-email-felipe.contreras@gmail.com> URL: https://gitlist.dev/e/1367093407-8898-7-git-send-email-felipe.contreras%40gmail.com In-Reply-To: <1367093407-8898-1-git-send-email-felipe.contreras@gmail.com> ``` The calls to __gitcomp_file() are essentially the same, but with different prefix. Signed-off-by: Felipe Contreras --- contrib/completion/git-completion.bash | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 2561265..9cea170 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -535,20 +535,17 @@ __git_complete_revlist_file () # The exception is --committable, which finds the files appropriate commit. __git_complete_index_file () { - local pfx cur_="$cur" + local pfx="" cur_="$cur" case "$cur_" in ?*/*) pfx="${cur_%/*}" cur_="${cur_##*/}" pfx="${pfx}/" - - __gitcomp_file "$(__git_index_files "$1" "$pfx")" "$pfx" "$cur_" - ;; - *) - __gitcomp_file "$(__git_index_files "$1")" "" "$cur_" ;; esac + + __gitcomp_file "$(__git_index_files "$1" "$pfx")" "$pfx" "$cur_" } __git_complete_file () -- 1.8.2.1.1031.g2ee5873 ``` ## Felipe Contreras, 2013-04-27 20:10 Subject: [PATCH v2 7/9] completion: add hack to enable file mode in bash < 4 Message-ID: <1367093407-8898-8-git-send-email-felipe.contreras@gmail.com> URL: https://gitlist.dev/e/1367093407-8898-8-git-send-email-felipe.contreras%40gmail.com In-Reply-To: <1367093407-8898-1-git-send-email-felipe.contreras@gmail.com> ``` This way we don't need all the compat stuff, different filters, and so on. Also, now we complete exactly the same in bash 3 and bash 4. This is the way bash-completion did it for quite some time, when bash 3 was supported. For more information about the hack: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=272660#64 Signed-off-by: Felipe Contreras --- contrib/completion/git-completion.bash | 43 ++++++---------------------------- 1 file changed, 7 insertions(+), 36 deletions(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 9cea170..f9e8e7d 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -254,33 +254,21 @@ __gitcomp_file () # completion will be used. __gitcompadd "$1" "${2-}" "${3-$cur}" "" - # Tell Bash that compspec generates filenames. - compopt -o filenames 2>/dev/null + # use a hack to enable file mode in bash < 4 + compopt -o filenames 2>/dev/null || + compgen -f /non-existing-dir/ > /dev/null } -__git_index_file_list_filter_compat () -{ - local path - - while read -r path; do - case "$path" in - ?*/*) echo "${path%%/*}/" ;; - *) echo "$path" ;; - esac - done -} - -__git_index_file_list_filter_bash () +# Process path list returned by "ls-files" and "diff-index --name-only" +# commands, in order to list only file names relative to a specified +# directory, and append a slash to directory names. +__git_index_file_list_filter () { local path while read -r path; do case "$path" in ?*/*) - # XXX if we append a slash to directory names when using - # `compopt -o filenames`, Bash will append another slash. - # This is pretty stupid, and this the reason why we have to - # define a compatible version for this function. echo "${path%%/*}" ;; *) echo "$path" ;; @@ -288,15 +276,6 @@ __git_index_file_list_filter_bash () done } -# Process path list returned by "ls-files" and "diff-index --name-only" -# commands, in order to list only file names relative to a specified -# directory, and append a slash to directory names. -__git_index_file_list_filter () -{ - # Default to Bash >= 4.x - __git_index_file_list_filter_bash -} - # Execute 'git ls-files', unless the --committable option is specified, in # which case it runs 'git diff-index' to find out the files that can be # committed. It return paths relative to the directory specified in the first @@ -2651,14 +2630,6 @@ if [[ -n ${ZSH_VERSION-} ]]; then compdef _git git gitk return -elif [[ -n ${BASH_VERSION-} ]]; then - if ((${BASH_VERSINFO[0]} < 4)); then - # compopt is not supported - __git_index_file_list_filter () - { - __git_index_file_list_filter_compat - } - fi fi __git_func_wrap () -- 1.8.2.1.1031.g2ee5873 ``` ## Felipe Contreras, 2013-04-27 20:10 Subject: [PATCH v2 8/9] completion: add space after completed filename Message-ID: <1367093407-8898-9-git-send-email-felipe.contreras@gmail.com> URL: https://gitlist.dev/e/1367093407-8898-9-git-send-email-felipe.contreras%40gmail.com In-Reply-To: <1367093407-8898-1-git-send-email-felipe.contreras@gmail.com> ``` Just like before fea16b4 (git-completion.bash: add support for path completion). Signed-off-by: Felipe Contreras --- contrib/completion/git-completion.bash | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index f9e8e7d..20c9718 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -255,7 +255,7 @@ __gitcomp_file () __gitcompadd "$1" "${2-}" "${3-$cur}" "" # use a hack to enable file mode in bash < 4 - compopt -o filenames 2>/dev/null || + compopt -o filenames +o nospace 2>/dev/null || compgen -f /non-existing-dir/ > /dev/null } -- 1.8.2.1.1031.g2ee5873 ``` ## Felipe Contreras, 2013-04-27 20:10 Subject: [PATCH v2 9/9] completion: remove __git_index_file_list_filter() Message-ID: <1367093407-8898-10-git-send-email-felipe.contreras@gmail.com> URL: https://gitlist.dev/e/1367093407-8898-10-git-send-email-felipe.contreras%40gmail.com In-Reply-To: <1367093407-8898-1-git-send-email-felipe.contreras@gmail.com> ``` Refactor the code into the only caller; __git_index_files(). Also, Somehow messing up with the 'path' variable messes up the 'PATH' variable. So let's not do that. Signed-off-by: Felipe Contreras --- contrib/completion/git-completion.bash | 28 ++++++++-------------------- 1 file changed, 8 insertions(+), 20 deletions(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 20c9718..edb7428 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -259,23 +259,6 @@ __gitcomp_file () compgen -f /non-existing-dir/ > /dev/null } -# Process path list returned by "ls-files" and "diff-index --name-only" -# commands, in order to list only file names relative to a specified -# directory, and append a slash to directory names. -__git_index_file_list_filter () -{ - local path - - while read -r path; do - case "$path" in - ?*/*) - echo "${path%%/*}" ;; - *) - echo "$path" ;; - esac - done -} - # Execute 'git ls-files', unless the --committable option is specified, in # which case it runs 'git diff-index' to find out the files that can be # committed. It return paths relative to the directory specified in the first @@ -303,11 +286,16 @@ __git_ls_files_helper () # slash. __git_index_files () { - local dir="$(__gitdir)" root="${2-.}" + local dir="$(__gitdir)" root="${2-.}" file if [ -d "$dir" ]; then - __git_ls_files_helper "$root" "$1" | __git_index_file_list_filter | - sort | uniq + __git_ls_files_helper "$root" "$1" | + while read -r file; do + case "$file" in + ?*/*) echo "${file%%/*}" ;; + *) echo "$file" ;; + esac + done | sort | uniq fi } -- 1.8.2.1.1031.g2ee5873 ``` ## Junio C Hamano, 2013-04-27 21:24 Subject: Re: [PATCH v2 1/9] completion: add file completion tests Message-ID: <7vehdvu0og.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vehdvu0og.fsf%40alter.siamese.dyndns.org In-Reply-To: <1367093407-8898-2-git-send-email-felipe.contreras@gmail.com> ``` Felipe Contreras writes: > The commit fea16b4 (git-completion.bash: add support for path > completion) introduced quite a few changes, without the usual tests. > > Signed-off-by: Felipe Contreras > --- > t/t9902-completion.sh | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 68 insertions(+) > > diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh > index 6d9d141..385e1e4 100755 > --- a/t/t9902-completion.sh > +++ b/t/t9902-completion.sh > @@ -347,4 +347,72 @@ test_expect_success 'send-email' ' > test_completion "git send-email ma" "master " > ' > > +test_expect_success 'complete files' ' > + git init tmp && cd tmp && > + test_when_finished "cd .. && rm -rf tmp" && Hmm, this construct is not very nice. Later part of the code can be modified carelessly and chdir around even more, and "cd .." may end up at an unexpected place. Can't this be done in a way to make later reviews of patches that come after this change lands doable without looking at the whole 70 lines? For example, if the test ran the whole "these are done in a subdirectry" part, starting from the "cd tmp", inside a subshell, and "rm -rf tmp" is done in the original shell that ran "git init tmp" to create it, it would make it much harder to make mistakes for anybody who add new code to the body of the test to affect the clean-up part. > + > + echo "expected" > .gitignore && > + echo "out" >> .gitignore && Style: we tend to omit SP between redireciton operator and targets. > + git add .gitignore && > + test_completion "git commit " ".gitignore" && > + > + git commit -m ignore && > + > + touch new && > + test_completion "git add " "new" && > + > + git add new && > + git commit -a -m new && > + test_completion "git add " "" && > + > + git mv new modified && > + echo modify > modified && > + test_completion "git add " "modified" && > + > + touch untracked && > + > + : TODO .gitignore should not be here && > + test_completion "git rm " <<-\EOF && > + .gitignore > + modified > + EOF > + > + test_completion "git clean " "untracked" && > + > + : TODO .gitignore should not be here && > + test_completion "git mv " <<-\EOF && > + .gitignore > + modified > + EOF > + > + mkdir dir && > + touch dir/file-in-dir && > + git add dir/file-in-dir && > + git commit -m dir && > + > + mkdir untracked-dir && > + > + : TODO .gitignore should not be here && > + test_completion "git mv modified " <<-\EOF && > + .gitignore > + dir > + modified > + untracked > + untracked-dir > + EOF > + > + test_completion "git commit " "modified" && > + > + : TODO .gitignore should not be here && > + test_completion "git ls-files " <<-\EOF > + .gitignore > + dir > + modified > + EOF > + > + touch momified && > + test_completion "git add mom" "momified" > +' > + > test_done ``` ## Junio C Hamano, 2013-04-27 21:29 Subject: Re: [PATCH v2 5/9] completion: refactor diff_index wrappers Message-ID: <7v7gjnu0gd.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7v7gjnu0gd.fsf%40alter.siamese.dyndns.org In-Reply-To: <1367093407-8898-6-git-send-email-felipe.contreras@gmail.com> ``` Felipe Contreras writes: > At the end of the day what we really need is to find out the files that > have been staged, or modified, because those files are the ones that > make sense to pass as arguments to 'git commit'. > > We need diff-index to find those out, since 'git ls-files' doesn't do > that. > > But we don't need wrappers and wrappers basically identical to the ones > used for 'git ls-files', when we can pretend it receives a --committable > option that would return what we need. > > That way, we can remove a bunch of code without any functional changes. > > Signed-off-by: Felipe Contreras > --- > contrib/completion/git-completion.bash | 71 ++++++++-------------------------- > 1 file changed, 16 insertions(+), 55 deletions(-) Nice line reduction. > > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash > index 7f1ebe4..2561265 100644 > --- a/contrib/completion/git-completion.bash > +++ b/contrib/completion/git-completion.bash > @@ -297,30 +297,25 @@ __git_index_file_list_filter () > __git_index_file_list_filter_bash > } > > -# Execute git ls-files, returning paths relative to the directory > -# specified in the first argument, and using the options specified in > -# the second argument. > +# Execute 'git ls-files', unless the --committable option is specified, in > +# which case it runs 'git diff-index' to find out the files that can be > +# committed. It return paths relative to the directory specified in the first > +# argument, and using the options specified in the second argument. > __git_ls_files_helper () > { > ( > test -n "${CDPATH+set}" && unset CDPATH > - # NOTE: $2 is not quoted in order to support multiple options > - cd "$1" && git ls-files --exclude-standard $2 > + cd "$1" > + if [ "$2" == "--committable" ]; then > + git diff-index --name-only --relative HEAD > + else > + # NOTE: $2 is not quoted in order to support multiple options > + git ls-files --exclude-standard $2 > + fi > ) 2>/dev/null > } > > > -# Execute git diff-index, returning paths relative to the directory > -# specified in the first argument, and using the tree object id > -# specified in the second argument. > -__git_diff_index_helper () > -{ > - ( > - test -n "${CDPATH+set}" && unset CDPATH > - cd "$1" && git diff-index --name-only --relative "$2" > - ) 2>/dev/null > -} > - > # __git_index_files accepts 1 or 2 arguments: > # 1: Options to pass to ls-files (required). > # 2: A directory path (optional). > @@ -337,22 +332,6 @@ __git_index_files () > fi > } > > -# __git_diff_index_files accepts 1 or 2 arguments: > -# 1) The id of a tree object. > -# 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. > -__git_diff_index_files () > -{ > - local dir="$(__gitdir)" root="${2-.}" > - > - if [ -d "$dir" ]; then > - __git_diff_index_helper "$root" "$1" | __git_index_file_list_filter | > - sort | uniq > - fi > -} > - > __git_heads () > { > local dir="$(__gitdir)" > @@ -550,8 +529,10 @@ __git_complete_revlist_file () > } > > > -# __git_complete_index_file requires 1 argument: the options to pass to > -# ls-file > +# __git_complete_index_file requires 1 argument: > +# 1: the options to pass to ls-file > +# > +# The exception is --committable, which finds the files appropriate commit. > __git_complete_index_file () > { > local pfx cur_="$cur" > @@ -570,26 +551,6 @@ __git_complete_index_file () > esac > } > > -# __git_complete_diff_index_file requires 1 argument: the id of a tree > -# object > -__git_complete_diff_index_file () > -{ > - local pfx cur_="$cur" > - > - case "$cur_" in > - ?*/*) > - pfx="${cur_%/*}" > - cur_="${cur_##*/}" > - pfx="${pfx}/" > - > - __gitcomp_file "$(__git_diff_index_files "$1" "$pfx")" "$pfx" "$cur_" > - ;; > - *) > - __gitcomp_file "$(__git_diff_index_files "$1")" "" "$cur_" > - ;; > - esac > -} > - > __git_complete_file () > { > __git_complete_revlist_file > @@ -1211,7 +1172,7 @@ _git_commit () > esac > > if git rev-parse --verify --quiet HEAD >/dev/null; then > - __git_complete_diff_index_file "HEAD" > + __git_complete_index_file "--committable" > else > # This is the first commit > __git_complete_index_file "--cached" ``` ## Junio C Hamano, 2013-04-27 21:34 Subject: Re: [PATCH v2 0/9] completion: general cleanups Message-ID: <7v38ubu087.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7v38ubu087.fsf%40alter.siamese.dyndns.org In-Reply-To: <1367093407-8898-1-git-send-email-felipe.contreras@gmail.com> ``` Overall this looks like a nice clean-up. I didn't look at the 5/9 carefully to verify its claim that the change is a no-op, though. Will queue. ``` ## Eric Sunshine, 2013-04-28 01:41 Subject: Re: [PATCH v2 9/9] completion: remove __git_index_file_list_filter() Message-ID: URL: https://gitlist.dev/e/CAPig%2BcSp2wsvZaJ1vKn3UOGL4FyGFj4j4OFKQAeN-pVNMYZKaw%40mail.gmail.com In-Reply-To: <1367093407-8898-10-git-send-email-felipe.contreras@gmail.com> ``` On Sat, Apr 27, 2013 at 4:10 PM, Felipe Contreras wrote: > Refactor the code into the only caller; __git_index_files(). > > Also, Somehow messing up with the 'path' variable messes up the 'PATH' s/Somehow/somehow/ s/messing up/messing/ > variable. So let's not do that. > > Signed-off-by: Felipe Contreras ``` ## Felipe Contreras, 2013-04-28 01:45 Subject: Re: [PATCH v2 9/9] completion: remove __git_index_file_list_filter() Message-ID: URL: https://gitlist.dev/e/CAMP44s0kqMVFF-PGJSiRJ9TYpA8UQrdbEUumMRuL%3DFDGtg0B3A%40mail.gmail.com In-Reply-To: ``` On Sat, Apr 27, 2013 at 8:41 PM, Eric Sunshine wrote: > On Sat, Apr 27, 2013 at 4:10 PM, Felipe Contreras > wrote: >> Refactor the code into the only caller; __git_index_files(). >> >> Also, Somehow messing up with the 'path' variable messes up the 'PATH' > > s/Somehow/somehow/ > s/messing up/messing/ More important than that is the fact that the problem is only on zsh. -- Felipe Contreras ```