Hi, On Wed, Aug 31, 2011 at 04:43:03AM +0700, Ron Panduwana wrote: > Signed-off-by: Ron Panduwana <panduwana@xxxxxxxxx> I agree with Junio that some explanation would be necessary here. I'm not sure this change is in general a good idea. I can imagine that it would be useful for e.g. java projects, where you are forced to have deep directory structures, but it will surely cause surprises and confusion for users who trained their fingers to quickly enter long paths with a few keystrokes and completion. Furthermore, it seems to cause considerable performance penalty, because it will fork three processes and a subshell to connect them with a pipe for such a "simple" thing as file name completion. And one of those processes is git status, which alone can take a while, especially for large repositories, and as a side effect it refreshes the index (but that is probably harmless). > --- > > On Fri, Aug 19, 2011 at 5:10 PM, Thomas Rast <trast@xxxxxxxxxxxxxxx> wrote: > > Some thoughts: > > > > * running git-status for . has some issues: it doesn't work in the > > Â case of > > > > Â Â cd subdir > > Â Â git add ../some/file[TAB] > > > > Â It's also inefficient if you are at the top level and > > > > Â Â git add path/to/file/a/few/levels/down[TAB] > > > > Â since it wouldn't actually have to look for untracked files in the > > Â entire repo. > > Fixed by running git-status for $cur if $cur is a directory. Otherwise run on . That won't do. Imaginge you want to add ../some/file, and therefore you do 'git add ../so<TAB>'. In this case $cur will hold '../so', which is not a directory (or at least not the directory you are looking for). > > * -uall is not required unless you are looking for untracked files. > > Â For the other commands you could speed up completion by passing > > Â -uno instead. > > Fixed by adding second parameter to __git_files_having_status > > > contrib/completion/git-completion.bash | 84 ++++++++++++++++++++----------- > 1 files changed, 54 insertions(+), 30 deletions(-) > > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash > index 8648a36..9d44501 100755 > --- a/contrib/completion/git-completion.bash > +++ b/contrib/completion/git-completion.bash > @@ -1010,6 +1010,16 @@ __git_has_doubledash () > return 1 > } > > +# __git_files_having_status requires 2 arguments > +__git_files_having_status () > +{ > + local dir="." > + if [ -d "$cur" ]; then > + dir="$cur" > + fi Please use tabs for indentation. > + echo "$(git status $2 -s "$dir" 2>/dev/null | egrep "^$1" | cut -c4-)" Echoing a command substitution is unnecessary. > +} > + > __git_whitespacelist="nowarn warn error error-all fix" > > _git_am () > @@ -1058,17 +1068,17 @@ _git_apply () > > _git_add () > { > - __git_has_doubledash && return > - > - case "$cur" in > - --*) > - __gitcomp " > - --interactive --refresh --patch --update --dry-run > - --ignore-errors --intent-to-add > - " > - return > - esac > - COMPREPLY=() > + if ! __git_has_doubledash; then > + case "$cur" in > + --*) > + __gitcomp " > + --interactive --refresh --patch --update --dry-run > + --ignore-errors --intent-to-add > + " > + return > + esac > + fi > + __gitcomp "$(__git_files_having_status "(.[MAU]|UD|\?\?)" -uall)" Please do this and the others the other way around, as you did in _git_checkout(), i.e. if __git_has_doubledash ; then __gitcomp "$(__git_files_having_status ...)" return fi case "$cur" in [...] This will make the patch shorter and easier to review, it won't increase the indentation unnecesarily, and it will be friendlier to the users running 'git blame' in the future. > } > > _git_archive () > @@ -1171,7 +1181,12 @@ _git_bundle () > > _git_checkout () > { > - __git_has_doubledash && return > + if __git_has_doubledash; then > + if [[ ${words[2]} = "--" ]]; then > + __gitcomp "$(__git_files_having_status ".[MD]" -uno)" > + fi > + return > + fi > > case "$cur" in > --conflict=*) > @@ -1469,7 +1484,7 @@ _git_help () > __gitcomp "$__git_all_commands $(__git_aliases) > attributes cli core-tutorial cvs-migration > diffcore gitk glossary hooks ignore modules > - namespaces repository-layout tutorial tutorial-2 > + repository-layout tutorial tutorial-2 That's an independent change; I don't think you want to remove namespaces here. > workflows > " > } > @@ -2313,14 +2328,18 @@ _git_replace () > > _git_reset () > { > - __git_has_doubledash && return > - > - case "$cur" in > - --*) > - __gitcomp "--merge --mixed --hard --soft --patch" > + if ! __git_has_doubledash; then > + case "$cur" in > + --*) > + __gitcomp "--merge --mixed --hard --soft --patch" > + return > + ;; > + esac > + fi > + if [[ ${words[2]} = "HEAD" ]]; then > + __gitcomp "$(__git_files_having_status "[ADM]." -uno)" > return > - ;; > - esac > + fi > __gitcomp "$(__git_refs)" > } > > @@ -2337,15 +2356,20 @@ _git_revert () > > _git_rm () > { > - __git_has_doubledash && return > - > - case "$cur" in > - --*) > - __gitcomp "--cached --dry-run --ignore-unmatch --quiet" > - return > - ;; > - esac > - COMPREPLY=() > + if ! __git_has_doubledash; then > + case "$cur" in > + --*) > + __gitcomp "--cached --dry-run --ignore-unmatch --quiet" > + return > + ;; > + esac > + fi > + # check if --cached was specified > + if [ "$(__git_find_on_cmdline "--cached")" ]; then > + COMPREPLY=() > + else > + __gitcomp "$(__git_files_having_status "(.D|DU|UA)" -uno)" 'git rm' can be used to delete any tracked files, but this limits completion to only those files that are already deleted from the work tree or are unmerged. > + fi > } > > _git_shortlog () > @@ -2640,7 +2664,6 @@ _git () > --exec-path > --html-path > --work-tree= > - --namespace= Independent change. > --help > " > ;; > @@ -2737,3 +2760,4 @@ else > shopt "$@" > } > fi > + Superfluous new line. Best, Gábor -- 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