Slowness issue with git completion

Slowness issue with git completion

[Felipe Contreras]

Hi,

It's very easy to reproduce:
% git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
linux
% cd linux
% git log v<tab>

It will take a looong time to figure out anything, specially if not
cached. I think I recall investigating the issue and finding that it's
looking for *all* the files in the git repo.

Cheers.

-- 
Felipe Contreras

Re: Slowness issue with git completion

[Frank Terbeck]

Felipe Contreras wrote:
> It's very easy to reproduce:
> % git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
> linux
> % cd linux
> % git log v<tab>
>
> It will take a looong time to figure out anything, specially if not
> cached. I think I recall investigating the issue and finding that it's
> looking for *all* the files in the git repo.

Well yes. This is a known issue.

I'm fairly sure that this is due to _multi_parts, which probably doesn't
scale for jobs like that.  I once gave rewriting all that a shot and
ended up with a number of helpers for each "type" of file etc. (like
modified or ignored files).  While they were significantly quicker than
the current code, they were probably still full of bugs and
shortcomings. Also, I would have had to rewrite *large* parts of the
rest of the completion, which would have been a *major* undertaking.

In short: It's a known issue and it's very hard to fix (if only because
it is a *lot* of work).

Regards, Frank

-- 
In protocol design, perfection has been reached not when there is
nothing left to add, but when there is nothing left to take away.
                                                  -- RFC 1925

Re: Slowness issue with git completion

[Nikolai Weibull]

On Tue, Apr 26, 2011 at 20:43, Frank Terbeck <ft@bewatermyfriend.org> wrote:
> Felipe Contreras wrote:
>> It's very easy to reproduce:
>> % git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
>> linux
>> % cd linux
>> % git log v<tab>
>>
>> It will take a looong time to figure out anything, specially if not
>> cached. I think I recall investigating the issue and finding that it's
>> looking for *all* the files in the git repo.
>
> Well yes. This is a known issue.
>
> I'm fairly sure that this is due to _multi_parts, which probably doesn't
> scale for jobs like that.  I once gave rewriting all that a shot and
> ended up with a number of helpers for each "type" of file etc. (like
> modified or ignored files).  While they were significantly quicker

How did you rewrite it?  I tried implementing it with
*(e:__git_cached:) (or similar), but that was, in my implementation, a
lot slower.  And this wasn’t even on Cygwin, where the forking makes
it even slower.

Re: Slowness issue with git completion

[Frank Terbeck]

Nikolai Weibull wrote:
> How did you rewrite it?  I tried implementing it with
> *(e:__git_cached:) (or similar), but that was, in my implementation, a
> lot slower.  And this wasn’t even on Cygwin, where the forking makes
> it even slower.

Below is how I was going about it. As I said, it may be full of bugs and
other short-comings.  It was pretty quick in the linux kernel repo,
though (I had implemented a minimal _git completion that used the
"git-add" completion to test the code).

I was also thinking about a scheme to give the user the option to choose
smart (and slow) and utterly-dumb (and quick) file completion in various
contexts via zstyle, but lost interest because it's just *so* much to
do.

Anyway, here it is:

# helpers...
# If a sub-command completion requires zsh to be in a git repository when
# executing, this function should be run to guard that. Usage:
#     __git-needs_repo || return 0
function __git-needs_repo {
    if [[ -z ${_gitdir} ]]; then
        _message "Not a git repository."
        return 1
    fi
    return 0
}

# Git file listing helpers.
#
# With git, files can be in various states. Files from one state may make
# sense for completion with one sub-command, but may be utterly useless
# with the next one. Therefore, we got a whole battery of helpers, that
# should make the job a lot easier.
#
# Identifying the enemy:
#
#   index files
#       Files git already knows about (aka. "stuff checked in"). The
#       "git ls-tree" command can list those.
#
#   modified files
#       Files git knows about, but which have also changed. These changes
#       could be staged or not. So we end up with the following set of
#       states:
#           mod-staged     - files with staged modifications
#               "git diff-index --cached --diff-filter=M --name-only"
#           mod-unstaged   - files, whose modifications are *not* staged
#               "git ls-files -m"
#           modified       - both.
#               "git diff-index --diff-filter=M --name-only"
#
#   other files
#       Git doesn't know these files. "git ls-files -o"
#
#   ignored files
#       Git doesn't know these, and it was even told to ignore them, when
#       it sees them. "git ls-files --exclude-standard -i -o"
#
#   deleted files
#       Files git knows about, but which will be removed from its knowledge
#       in the next commit (note that the actual file does not have to be
#       deleted - "git rm --cached file" has that effect).
#       "git diff-index --name-only --diff-filter=D"
#
#   killed files
#       Sometimes, in order to call "git checkout-index" successfully,
#       files or directories may need to be removed to resolve conflicts.
#       This may happen rarely, but we can use "git ls-files -k" to catch
#       these, so we probably should. It may come in handy.
#
#   unmerged files
#       When multiple versions of a file exist in the staging area (the
#       index file) that's an unmerged file. The only way I can think
#       of end up having these in git-completion is during the resolution
#       of a merge-conflict. And then, it would probably be most useful
#       to have these available in an editor's completion function. Like
#       "killed files", this sort of file is available from "git ls-files",
#       but it'll need extra filtering, because --unmerged implicitly
#       turns on --stage. "git ls-files -u"
#
# I'll be nameing these `__git-files_<type>', so ie "index files" will be
# completed by `__git-files_index'.

function __git-files_index {
    local expl cw cd
    local -a files

    cw=${words[CURRENT]}
    case $cw in
    (*/) cd=${cw} ;;
    (*/*) cd=${cw:h} ;;
    esac
    files=( ${(ps:\0:)"$(_call_program index-files git ls-tree --name-only -z HEAD${cd:+:$cd})"} )
    _wanted 'index-files' expl 'index file' compadd -f ${expl} -- ${cd:+$cd}"${files[@]}"
}

function __git-files_modified {
    local expl cw cd
    local -a files

    cw=${words[CURRENT]}
    case $cw in
    (*/) cd=${cw} ;;
    (*/*) cd=${cw:h} ;;
    esac
    files=( ${(ps:\0:)"$(_call_program modified-files git diff-index -z --diff-filter=MDA --name-only HEAD ${cd:+$cd})"} )
    files=( ${(u)${${(M)files:#${cd}*}/(#b)(${cd}[^\/]##)*/${match[1]}}} )
    _wanted 'modified-files' expl 'modified file' compadd -f ${expl} -- "${files[@]}"
}

function __git-files_modified_staged {
    local expl cw cd
    local -a files

    cw=${words[CURRENT]}
    case $cw in
    (*/) cd=${cw} ;;
    (*/*) cd=${cw:h} ;;
    esac
    files=( ${(ps:\0:)"$(_call_program staged-modified-files git diff-index -z --cached --diff-filter=MDA --name-only HEAD ${cd:+$cd})"} )
    files=( ${(u)${${(M)files:#${cd}*}/(#b)(${cd}[^\/]##)*/${match[1]}}} )
    _wanted 'staged-modified-files' expl 'staged modified file' compadd -f ${expl} -- "${files[@]}"
}

function __git-files_modified_unstaged {
    local expl cw cd
    local -a files

    cw=${words[CURRENT]}
    case $cw in
    (*/) cd=${cw} ;;
    (*/*) cd=${cw:h} ;;
    esac
    files=( ${(ps:\0:)"$(_call_program unstaged-modified-files git ls-files --exclude-standard -m -z ${cd:+$cd})"} )
    files=( ${(u)${${(M)files:#${cd}*}/(#b)(${cd}[^\/]##)*/${match[1]}}} )
    _wanted 'unstaged-modified-files' expl 'unstaged modified file' compadd -f ${expl} -- "${files[@]}"
}

function __git-files_ignored {
    local expl cw cd
    local -a files

    cw=${words[CURRENT]}
    case $cw in
    (*/) cd=${cw} ;;
    (*/*) cd=${cw:h} ;;
    esac
    files=( ${(ps:\0:)"$(_call_program ignored-files git ls-files --exclude-standard -m -z ${cd:+$cd})"} )
    files=( ${(u)${${(M)files:#${cd}*}/(#b)(${cd}[^\/]##)*/${match[1]}}} )
    _wanted 'ignored-files' expl 'ignored file' compadd -f ${expl} -- "${files[@]}"
}

function __git-files_other {
    local expl cw cd
    local -a files

    cw=${words[CURRENT]}
    case $cw in
    (*/) cd=${cw} ;;
    (*/*) cd=${cw:h} ;;
    esac
    files=( ${(ps:\0:)"$(_call_program other-files git ls-files --exclude-standard -o -z ${cd:+$cd})"} )
    files=( ${(u)${${(M)files:#${cd}*}/(#b)(${cd}[^\/]##)*/${match[1]}}} )
    _wanted 'other-files' expl 'other file' compadd -f ${expl} -- "${files[@]}"
}

function __git-files_deleted {
    local expl cw cd
    local -a files

    cw=${words[CURRENT]}
    case $cw in
    (*/) cd=${cw} ;;
    (*/*) cd=${cw:h} ;;
    esac
    files=( ${(ps:\0:)"$(_call_program deleted-files git diff-index -z --diff-filter=D --name-only HEAD ${cd:+$cd})"} )
    files=( ${(u)${${(M)files:#${cd}*}/(#b)(${cd}[^\/]##)*/${match[1]}}} )
    _wanted 'deleted-files' expl 'deleted file' compadd -f ${expl} -- "${files[@]}"
}

# other helpers
function __git-tags {
    local expl cw
    local -a tags

    cw=${words[CURRENT]}
    tags=( ${${(f)"$(_call_program git-tags git for-each-ref --format='"%(refname)"' refs/tags/${cw:+$cw*})"}#refs/tags/} )
    _wanted 'tags' expl 'git tag' compadd ${expl} -- "${tags[@]}"
}

function __git-branches {
    local expl cw
    local -a branches

    cw=${words[CURRENT]}
    branches=( ${${(f)"$(_call_program git-branches git for-each-ref --format='"%(refname)"' refs/heads/${cw:+$cw*})"}#refs/heads/} )
    _wanted 'branches' expl 'git branch' compadd ${expl} -- "${branches[@]}"
}

function __git-branches_remote {
    local expl cw
    local -a branches

    cw=${words[CURRENT]}
    branches=( ${${(f)"$(_call_program git-branches-remote git for-each-ref --format='"%(refname)"' refs/remotes/${cw:+$cw*})"}#refs/remotes/} )
    _wanted 'remote branches' expl 'git branch (remote)' compadd ${expl} -- "${branches[@]}"
}

function __git-remotes {
    local expl cw
    local -a remotes

    cw=${words[CURRENT]}
    remotes=( ${${${${(f)"$(_call_program git-remotes git config --get-regexp "'remote\..*\.url'")"}%% *}#remote.}%.url} )
    _wanted 'remotes' expl 'remote' compadd ${expl} -- "${remotes[@]}"
}

Re: Slowness issue with git completion

[Felipe Contreras]

On Tue, Apr 26, 2011 at 9:43 PM, Frank Terbeck <ft@bewatermyfriend.org> wrote:
> Felipe Contreras wrote:
>> It's very easy to reproduce:
>> % git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
>> linux
>> % cd linux
>> % git log v<tab>
>>
>> It will take a looong time to figure out anything, specially if not
>> cached. I think I recall investigating the issue and finding that it's
>> looking for *all* the files in the git repo.
>
> Well yes. This is a known issue.
>
> I'm fairly sure that this is due to _multi_parts, which probably doesn't
> scale for jobs like that.  I once gave rewriting all that a shot and
> ended up with a number of helpers for each "type" of file etc. (like
> modified or ignored files).  While they were significantly quicker than
> the current code, they were probably still full of bugs and
> shortcomings. Also, I would have had to rewrite *large* parts of the
> rest of the completion, which would have been a *major* undertaking.
>
> In short: It's a known issue and it's very hard to fix (if only because
> it is a *lot* of work).

Here's a quick solution: don't try to list files. If I want to list
files, I would use '-- '. And in fact, that's exactly what the bash
completion does.

Moreover, why do you want to be smarter than git? There are ways to
ask git precisely what we want: list the contents of certain tree-ish
on-demand. And in fact, that's exactly what the bash completion does:

time git ls-tree HEAD -- (0m0.005s)
time git ls-tree HEAD -- drivers/ (0m0.007s)

Seriously, this is very annoying.

-- 
Felipe Contreras

Re: Slowness issue with git completion

[Mikael Magnusson]

On 26 April 2011 22:23, Felipe Contreras <felipe.contreras@gmail.com> wrote:
> On Tue, Apr 26, 2011 at 9:43 PM, Frank Terbeck <ft@bewatermyfriend.org> wrote:
>> Felipe Contreras wrote:
>>> It's very easy to reproduce:
>>> % git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
>>> linux
>>> % cd linux
>>> % git log v<tab>
>>>
>>> It will take a looong time to figure out anything, specially if not
>>> cached. I think I recall investigating the issue and finding that it's
>>> looking for *all* the files in the git repo.
>>
>> Well yes. This is a known issue.
>>
>> I'm fairly sure that this is due to _multi_parts, which probably doesn't
>> scale for jobs like that.  I once gave rewriting all that a shot and
>> ended up with a number of helpers for each "type" of file etc. (like
>> modified or ignored files).  While they were significantly quicker than
>> the current code, they were probably still full of bugs and
>> shortcomings. Also, I would have had to rewrite *large* parts of the
>> rest of the completion, which would have been a *major* undertaking.
>>
>> In short: It's a known issue and it's very hard to fix (if only because
>> it is a *lot* of work).
>
> Here's a quick solution: don't try to list files. If I want to list
> files, I would use '-- '. And in fact, that's exactly what the bash
> completion does.

git accepts files with or without a --

> Moreover, why do you want to be smarter than git? There are ways to
> ask git precisely what we want: list the contents of certain tree-ish
> on-demand. And in fact, that's exactly what the bash completion does:
>
> time git ls-tree HEAD -- (0m0.005s)
> time git ls-tree HEAD -- drivers/ (0m0.007s)

git ls-tree doesn't allow you to filter the output files by 'changed',
'new', 'unknown' etc etc.

> Seriously, this is very annoying.

This type of comment is not productive.

-- 
Mikael Magnusson

Re: Slowness issue with git completion

[Felipe Contreras]

On Tue, Apr 26, 2011 at 11:34 PM, Mikael Magnusson <mikachu@gmail.com> wrote:
> On 26 April 2011 22:23, Felipe Contreras <felipe.contreras@gmail.com> wrote:
>> On Tue, Apr 26, 2011 at 9:43 PM, Frank Terbeck <ft@bewatermyfriend.org> wrote:
>>> Felipe Contreras wrote:
>>>> It's very easy to reproduce:
>>>> % git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
>>>> linux
>>>> % cd linux
>>>> % git log v<tab>
>>>>
>>>> It will take a looong time to figure out anything, specially if not
>>>> cached. I think I recall investigating the issue and finding that it's
>>>> looking for *all* the files in the git repo.
>>>
>>> Well yes. This is a known issue.
>>>
>>> I'm fairly sure that this is due to _multi_parts, which probably doesn't
>>> scale for jobs like that.  I once gave rewriting all that a shot and
>>> ended up with a number of helpers for each "type" of file etc. (like
>>> modified or ignored files).  While they were significantly quicker than
>>> the current code, they were probably still full of bugs and
>>> shortcomings. Also, I would have had to rewrite *large* parts of the
>>> rest of the completion, which would have been a *major* undertaking.
>>>
>>> In short: It's a known issue and it's very hard to fix (if only because
>>> it is a *lot* of work).
>>
>> Here's a quick solution: don't try to list files. If I want to list
>> files, I would use '-- '. And in fact, that's exactly what the bash
>> completion does.
>
> git accepts files with or without a --

So?

>> Moreover, why do you want to be smarter than git? There are ways to
>> ask git precisely what we want: list the contents of certain tree-ish
>> on-demand. And in fact, that's exactly what the bash completion does:
>>
>> time git ls-tree HEAD -- (0m0.005s)
>> time git ls-tree HEAD -- drivers/ (0m0.007s)
>
> git ls-tree doesn't allow you to filter the output files by 'changed',
> 'new', 'unknown' etc etc.

So?

>> Seriously, this is very annoying.
>
> This type of comment is not productive.

Well, I have been waiting a long time for this to be fixed, if this is
a known issue, and you already know it's very annoying, and there's no
plan to fix it, I can only assume that you don't care. So I guess I
should use the bash completion.

-- 
Felipe Contreras

Re: Slowness issue with git completion

[Mikael Magnusson]

On 26 April 2011 22:57, Felipe Contreras <felipe.contreras@gmail.com> wrote:
> On Tue, Apr 26, 2011 at 11:34 PM, Mikael Magnusson <mikachu@gmail.com> wrote:
>> On 26 April 2011 22:23, Felipe Contreras <felipe.contreras@gmail.com> wrote:
>>> On Tue, Apr 26, 2011 at 9:43 PM, Frank Terbeck <ft@bewatermyfriend.org> wrote:
>>>> Felipe Contreras wrote:
>>>>> It's very easy to reproduce:
>>>>> % git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
>>>>> linux
>>>>> % cd linux
>>>>> % git log v<tab>
>>>>>
>>>>> It will take a looong time to figure out anything, specially if not
>>>>> cached. I think I recall investigating the issue and finding that it's
>>>>> looking for *all* the files in the git repo.
>>>>
>>>> Well yes. This is a known issue.
>>>>
>>>> I'm fairly sure that this is due to _multi_parts, which probably doesn't
>>>> scale for jobs like that.  I once gave rewriting all that a shot and
>>>> ended up with a number of helpers for each "type" of file etc. (like
>>>> modified or ignored files).  While they were significantly quicker than
>>>> the current code, they were probably still full of bugs and
>>>> shortcomings. Also, I would have had to rewrite *large* parts of the
>>>> rest of the completion, which would have been a *major* undertaking.
>>>>
>>>> In short: It's a known issue and it's very hard to fix (if only because
>>>> it is a *lot* of work).
>>>
>>> Here's a quick solution: don't try to list files. If I want to list
>>> files, I would use '-- '. And in fact, that's exactly what the bash
>>> completion does.
>>
>> git accepts files with or without a --
>
> So?

So if the command works, we should complete it.

>>> Moreover, why do you want to be smarter than git? There are ways to
>>> ask git precisely what we want: list the contents of certain tree-ish
>>> on-demand. And in fact, that's exactly what the bash completion does:
>>>
>>> time git ls-tree HEAD -- (0m0.005s)
>>> time git ls-tree HEAD -- drivers/ (0m0.007s)
>>
>> git ls-tree doesn't allow you to filter the output files by 'changed',
>> 'new', 'unknown' etc etc.
>
> So?

So we can use ls-tree for log i guess, but not for much else, git add
will still be slow for example.

-- 
Mikael Magnusson

Re: Slowness issue with git completion

[Felipe Contreras]

On Tue, Apr 26, 2011 at 11:59 PM, Mikael Magnusson <mikachu@gmail.com> wrote:
> On 26 April 2011 22:57, Felipe Contreras <felipe.contreras@gmail.com> wrote:
>> On Tue, Apr 26, 2011 at 11:34 PM, Mikael Magnusson <mikachu@gmail.com> wrote:
>>> On 26 April 2011 22:23, Felipe Contreras <felipe.contreras@gmail.com> wrote:
>>>> On Tue, Apr 26, 2011 at 9:43 PM, Frank Terbeck <ft@bewatermyfriend.org> wrote:
>>>>> Felipe Contreras wrote:
>>>>>> It's very easy to reproduce:
>>>>>> % git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
>>>>>> linux
>>>>>> % cd linux
>>>>>> % git log v<tab>
>>>>>>
>>>>>> It will take a looong time to figure out anything, specially if not
>>>>>> cached. I think I recall investigating the issue and finding that it's
>>>>>> looking for *all* the files in the git repo.
>>>>>
>>>>> Well yes. This is a known issue.
>>>>>
>>>>> I'm fairly sure that this is due to _multi_parts, which probably doesn't
>>>>> scale for jobs like that.  I once gave rewriting all that a shot and
>>>>> ended up with a number of helpers for each "type" of file etc. (like
>>>>> modified or ignored files).  While they were significantly quicker than
>>>>> the current code, they were probably still full of bugs and
>>>>> shortcomings. Also, I would have had to rewrite *large* parts of the
>>>>> rest of the completion, which would have been a *major* undertaking.
>>>>>
>>>>> In short: It's a known issue and it's very hard to fix (if only because
>>>>> it is a *lot* of work).
>>>>
>>>> Here's a quick solution: don't try to list files. If I want to list
>>>> files, I would use '-- '. And in fact, that's exactly what the bash
>>>> completion does.
>>>
>>> git accepts files with or without a --
>>
>> So?
>
> So if the command works, we should complete it.

Even if it's so slow that it is unusable? I don't understand the urge
to have that, nobody seems to be complaining about that behavior with
the bash completion. Even git itself warns when you have a ref and a
file that are ambiguous; you need to manually add "--".

BTW. The actual behavior is: if there's a ref, go for refs, if there's
no ref, try the files. So 'git show v' first would show the version
refs, but 'git show vi' will show the 'virt' directory. I don't see
how that is so atrocious. You can still auto-complete commands that
work.

>>>> Moreover, why do you want to be smarter than git? There are ways to
>>>> ask git precisely what we want: list the contents of certain tree-ish
>>>> on-demand. And in fact, that's exactly what the bash completion does:
>>>>
>>>> time git ls-tree HEAD -- (0m0.005s)
>>>> time git ls-tree HEAD -- drivers/ (0m0.007s)
>>>
>>> git ls-tree doesn't allow you to filter the output files by 'changed',
>>> 'new', 'unknown' etc etc.
>>
>> So?
>
> So we can use ls-tree for log i guess, but not for much else, git add
> will still be slow for example.

Why do you need any git command at all to find completions for 'git
add'? Just use the normal local file completion.

-- 
Felipe Contreras

Re: Slowness issue with git completion

[Mikael Magnusson]

On 26 April 2011 23:10, Felipe Contreras <felipe.contreras@gmail.com> wrote:
> On Tue, Apr 26, 2011 at 11:59 PM, Mikael Magnusson <mikachu@gmail.com> wrote:
>>>> git accepts files with or without a --
>>>
>>> So?
>>
>> So if the command works, we should complete it.
>
> Even if it's so slow that it is unusable? I don't understand the urge
> to have that, nobody seems to be complaining about that behavior with
> the bash completion. Even git itself warns when you have a ref and a
> file that are ambiguous; you need to manually add "--".

It's only slow in repos as large as the linux one.

> BTW. The actual behavior is: if there's a ref, go for refs, if there's
> no ref, try the files. So 'git show v' first would show the version
> refs, but 'git show vi' will show the 'virt' directory. I don't see
> how that is so atrocious. You can still auto-complete commands that
> work.
>
>>>>> Moreover, why do you want to be smarter than git? There are ways to
>>>>> ask git precisely what we want: list the contents of certain tree-ish
>>>>> on-demand. And in fact, that's exactly what the bash completion does:
>>>>>
>>>>> time git ls-tree HEAD -- (0m0.005s)
>>>>> time git ls-tree HEAD -- drivers/ (0m0.007s)
>>>>
>>>> git ls-tree doesn't allow you to filter the output files by 'changed',
>>>> 'new', 'unknown' etc etc.
>>>
>>> So?
>>
>> So we can use ls-tree for log i guess, but not for much else, git add
>> will still be slow for example.
>
> Why do you need any git command at all to find completions for 'git
> add'? Just use the normal local file completion.

Because you don't want to complete unchanged files when adding?

-- 
Mikael Magnusson

Re: Slowness issue with git completion

[Felipe Contreras]

On Wed, Apr 27, 2011 at 12:13 AM, Mikael Magnusson <mikachu@gmail.com> wrote:
> On 26 April 2011 23:10, Felipe Contreras <felipe.contreras@gmail.com> wrote:
>> On Tue, Apr 26, 2011 at 11:59 PM, Mikael Magnusson <mikachu@gmail.com> wrote:
>>>>> git accepts files with or without a --
>>>>
>>>> So?
>>>
>>> So if the command works, we should complete it.
>>
>> Even if it's so slow that it is unusable? I don't understand the urge
>> to have that, nobody seems to be complaining about that behavior with
>> the bash completion. Even git itself warns when you have a ref and a
>> file that are ambiguous; you need to manually add "--".
>
> It's only slow in repos as large as the linux one.

No. It's slow everywhere, it's *dead* slow on Linux. And BTW, there's
thousands of developers working on Linux (the kernel), I guess we are
not important.

I just tried on one of my projects that has only 67 files, it took
almost two seconds. That's completely unacceptable; you are blowing
git's performance away.

>> BTW. The actual behavior is: if there's a ref, go for refs, if there's
>> no ref, try the files. So 'git show v' first would show the version
>> refs, but 'git show vi' will show the 'virt' directory. I don't see
>> how that is so atrocious. You can still auto-complete commands that
>> work.
>>
>>>>>> Moreover, why do you want to be smarter than git? There are ways to
>>>>>> ask git precisely what we want: list the contents of certain tree-ish
>>>>>> on-demand. And in fact, that's exactly what the bash completion does:
>>>>>>
>>>>>> time git ls-tree HEAD -- (0m0.005s)
>>>>>> time git ls-tree HEAD -- drivers/ (0m0.007s)
>>>>>
>>>>> git ls-tree doesn't allow you to filter the output files by 'changed',
>>>>> 'new', 'unknown' etc etc.
>>>>
>>>> So?
>>>
>>> So we can use ls-tree for log i guess, but not for much else, git add
>>> will still be slow for example.
>>
>> Why do you need any git command at all to find completions for 'git
>> add'? Just use the normal local file completion.
>
> Because you don't want to complete unchanged files when adding?

Didn't you just said "if the command works, we should complete it"?

But what is the real advantage? That only one file would be listed
instead of a couple? Well, that would be nice, if it didn't take 10
seconds to do that. I would rather pick the right file myself, that's
faster.

Moreover, if you really want that, it's much easer to first find the
local files, and _then_ find they have been unchanged.

time git ls-files --modified --other -z -- > /dev/null (0m7.640s)
time git ls-files --modified --other -z -- 'M*' > /dev/null (0m2.224s)

That's dropping caches.

Cheers.

-- 
Felipe Contreras

Re: Slowness issue with git completion

[Benjamin R. Haskell]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 3302 bytes --]

On Tue, 26 Apr 2011, Felipe Contreras wrote:

> On Tue, Apr 26, 2011 at 11:34 PM, Mikael Magnusson wrote:
>> On 26 April 2011 22:23, Felipe Contreras wrote:
>>> On Tue, Apr 26, 2011 at 9:43 PM, Frank Terbeck wrote:
>>>> Felipe Contreras wrote:
>>>>> It's very easy to reproduce:
>>>>> % git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
>>>>> linux
>>>>> % cd linux
>>>>> % git log v<tab>
>>>>>
>>>>> It will take a looong time to figure out anything, specially if 
>>>>> not cached. I think I recall investigating the issue and finding 
>>>>> that it's looking for *all* the files in the git repo.
>>>>
>>>> Well yes. This is a known issue.
>>>>
>>>> I'm fairly sure that this is due to _multi_parts, which probably 
>>>> doesn't scale for jobs like that.  I once gave rewriting all that a 
>>>> shot and ended up with a number of helpers for each "type" of file 
>>>> etc. (like modified or ignored files).  While they were 
>>>> significantly quicker than the current code, they were probably 
>>>> still full of bugs and shortcomings. Also, I would have had to 
>>>> rewrite *large* parts of the rest of the completion, which would 
>>>> have been a *major* undertaking.
>>>>
>>>> In short: It's a known issue and it's very hard to fix (if only 
>>>> because it is a *lot* of work).
>>>
>>> Here's a quick solution: don't try to list files. If I want to list 
>>> files, I would use '-- '. And in fact, that's exactly what the bash 
>>> completion does.
>>
>> git accepts files with or without a --
>
> So?

So if git accepts filenames with or without the '--', Zsh's completion 
should list filenames with or without the '--'.


>>> Moreover, why do you want to be smarter than git? There are ways to 
>>> ask git precisely what we want: list the contents of certain 
>>> tree-ish on-demand. And in fact, that's exactly what the bash 
>>> completion does:
>>>
>>> time git ls-tree HEAD -- (0m0.005s)
>>> time git ls-tree HEAD -- drivers/ (0m0.007s)
>>
>> git ls-tree doesn't allow you to filter the output files by 
>> 'changed', 'new', 'unknown' etc etc.
>
> So?

So 'ls-tree' doesn't list context-appropriate completions, which is what 
Zsh is providing.  So, 'ls-tree' can't provide the list of files Zsh 
needs.

The point is that the way bash does it ignores the context cues that Zsh 
(very-)inefficiently exploits.

If you don't care about the correctness, just modify __git_files to only 
complete anything if it's already seen '--'.

Or, far more simply, just disable __git_files entirely.  I have 
<Alt>+<,> bound to file completion partly so I can complete filenames 
even in places where it doesn't make sense -- or more often: where Zsh 
doesn't think it makes sense -- but partly for this workaround in 
particular.

In ~/.zshrc or wherever:

" map alt-, to complete files
zle -C complete-files complete-word _generic
zstyle ':completion:complete-files:*' completer _files
bindkey '^[,' complete-files

" disable git file completion because it's so slow
__git_files(){}

It's a workaround.  At some point, someone (probably someone 
participating in this thread) will figure out a proper solution.  But 
until that happens the workaround is better than breaking the context 
sensitivity of git completion just to make it as fast as bash.

-- 
Best,
Ben

Re: Slowness issue with git completion

[Nikolai Weibull]

On Tue, Apr 26, 2011 at 23:35, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> On Wed, Apr 27, 2011 at 12:13 AM, Mikael Magnusson <mikachu@gmail.com> wrote:

>> It's only slow in repos as large as the linux one.

> No. It's slow everywhere, it's *dead* slow on Linux. And BTW, there's
> thousands of developers working on Linux (the kernel), I guess we are
> not important.

So perhaps you can improve it then.  Or one of the thousands of
developers working on Linux (the kernel).  (I don’t believe that a
very large fraction of them actually use Zsh, though, or someone would
have already provided a patch.)  Or perhaps you could patch the kernel
to make it faster?

I’ve spent innumerable hours on making it correct.  I expect someone
to pitch in a couple of hours to make it fast.

I find your attitude in this thread rather offensive.

Re: Slowness issue with git completion

[Felipe Contreras]

On Wed, Apr 27, 2011 at 12:52 AM, Benjamin R. Haskell <zsh@benizi.com> wrote:
> On Tue, 26 Apr 2011, Felipe Contreras wrote:
>
>> On Tue, Apr 26, 2011 at 11:34 PM, Mikael Magnusson wrote:
>>>
>>> On 26 April 2011 22:23, Felipe Contreras wrote:
>>>>
>>>> On Tue, Apr 26, 2011 at 9:43 PM, Frank Terbeck wrote:
>>>>>
>>>>> Felipe Contreras wrote:
>>>>>>
>>>>>> It's very easy to reproduce:
>>>>>> % git clone
>>>>>> git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
>>>>>> linux
>>>>>> % cd linux
>>>>>> % git log v<tab>
>>>>>>
>>>>>> It will take a looong time to figure out anything, specially if not
>>>>>> cached. I think I recall investigating the issue and finding that it's
>>>>>> looking for *all* the files in the git repo.
>>>>>
>>>>> Well yes. This is a known issue.
>>>>>
>>>>> I'm fairly sure that this is due to _multi_parts, which probably
>>>>> doesn't scale for jobs like that.  I once gave rewriting all that a shot and
>>>>> ended up with a number of helpers for each "type" of file etc. (like
>>>>> modified or ignored files).  While they were significantly quicker than the
>>>>> current code, they were probably still full of bugs and shortcomings. Also,
>>>>> I would have had to rewrite *large* parts of the rest of the completion,
>>>>> which would have been a *major* undertaking.
>>>>>
>>>>> In short: It's a known issue and it's very hard to fix (if only because
>>>>> it is a *lot* of work).
>>>>
>>>> Here's a quick solution: don't try to list files. If I want to list
>>>> files, I would use '-- '. And in fact, that's exactly what the bash
>>>> completion does.
>>>
>>> git accepts files with or without a --
>>
>> So?
>
> So if git accepts filenames with or without the '--', Zsh's completion
> should list filenames with or without the '--'.

I already explained, you still can auto-complete; first you try the
refs, if there's no ref, then you try the files. Why should I wait 10
seconds for the files to be ready if I'm trying to complete a ref?

>>>> Moreover, why do you want to be smarter than git? There are ways to ask
>>>> git precisely what we want: list the contents of certain tree-ish on-demand.
>>>> And in fact, that's exactly what the bash completion does:
>>>>
>>>> time git ls-tree HEAD -- (0m0.005s)
>>>> time git ls-tree HEAD -- drivers/ (0m0.007s)
>>>
>>> git ls-tree doesn't allow you to filter the output files by 'changed',
>>> 'new', 'unknown' etc etc.
>>
>> So?
>
> So 'ls-tree' doesn't list context-appropriate completions, which is what Zsh
> is providing.  So, 'ls-tree' can't provide the list of files Zsh needs.

You don't need any "context-appropriate" completions for 'git log',
'git show' or anything similar to rev-list. All you need is the
tree-ish information that ls-tree provides.

For 'git add', zsh doesn't *need* that "context". It's just the way it
was implemented in zsh, it's just status quo. It can easily be changed
and nobody would notice.

> The point is that the way bash does it ignores the context cues that Zsh
> (very-)inefficiently exploits.
>
> If you don't care about the correctness, just modify __git_files to only
> complete anything if it's already seen '--'.

Nah.

> It's a workaround.  At some point, someone (probably someone participating
> in this thread) will figure out a proper solution.  But until that happens
> the workaround is better than breaking the context sensitivity of git
> completion just to make it as fast as bash.

I disagree. The "context sensitivity" doesn't work. The current
behavior is broken.

I say it should be the other way around, that "context sensitivity"
which is only used for 'git add' should not make the rest of the
commands dead slow, it should be disabled until it's working properly
(fast).

-- 
Felipe Contreras

Re: Slowness issue with git completion

[Felipe Contreras]

On Wed, Apr 27, 2011 at 1:03 AM, Nikolai Weibull <now@bitwi.se> wrote:
> On Tue, Apr 26, 2011 at 23:35, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> On Wed, Apr 27, 2011 at 12:13 AM, Mikael Magnusson <mikachu@gmail.com> wrote:
>
>>> It's only slow in repos as large as the linux one.
>
>> No. It's slow everywhere, it's *dead* slow on Linux. And BTW, there's
>> thousands of developers working on Linux (the kernel), I guess we are
>> not important.
>
> So perhaps you can improve it then.  Or one of the thousands of
> developers working on Linux (the kernel).  (I don’t believe that a
> very large fraction of them actually use Zsh,

And now I see why.

> though, or someone would
> have already provided a patch.)  Or perhaps you could patch the kernel
> to make it faster?

It works fine in bash.

> I’ve spent innumerable hours on making it correct.  I expect someone
> to pitch in a couple of hours to make it fast.

Well, kernel and git developers have spent many more hours to make git
fast,  and this "correctness" is disrespecting that work because the
time it takes to execute the completion is longer than the command
itself.

> I find your attitude in this thread rather offensive.

Oh, if I say the current implementation is dead slow and unusable, you
find that offensive? Maybe you should understand that criticism to
code is not criticism to you. But I will tell you what I do when
somebody finds a fatal flaw in my software; I fix it. If the person is
offensive or cordial doesn't really matter, the fatal flaw is there.

Now, how about you make a compromise between "correctness" and
usability? There's many parts of the code marked as TODO, so I guess
the code is not "perfectly correct". Add one more TODO, so in the
meantime 'git add' shows unchanged files, which would make the rest of
the commands as fast as they should be.

I will tell you why I use completion; because I want to be more
productive. If completion makes me less productive, I will obviously
not use it... What's the point? I don't see why that is so hard to
understand. Note that this is the case also for small projects.

Are you interested in fixing this use-case even if it means to make
some compromises in correctness or not?

-- 
Felipe Contreras

Re: Slowness issue with git completion

[Benjamin R. Haskell]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1345 bytes --]

On Wed, 27 Apr 2011, Felipe Contreras wrote:

> On Wed, Apr 27, 2011 at 12:52 AM, Benjamin R. Haskell wrote:
>> It's a workaround.  At some point, someone (probably someone 
>> participating in this thread) will figure out a proper solution.  But 
>> until that happens the workaround is better than breaking the context 
>> sensitivity of git completion just to make it as fast as bash.
>
> I disagree. The "context sensitivity" doesn't work. The current 
> behavior is broken.

Everyone else in the thread seems to think that it's desired.  That is: 
if it weren't unusably slow, it'd be preferable.


> I say it should be the other way around, that "context sensitivity" 
> which is only used for 'git add' should not make the rest of the 
> commands dead slow, it should be disabled until it's working properly 
> (fast).

Disabling the context sensitivity of Zsh's git file completion requires 
rewriting other portions of the completion, since the rest depends on 
being able to call it how it does.  People who are actively contributing 
to the _git function want this functionality.  So, from that perspective 
it's not worth the effort to rip it out.

I've given you an admittedly-stupid workaround.  The other options are 
to wait for someone to fix it, fix it yourself, or use bash (which has 
the behavior you want).

-- 
Best,
Ben

Re: Slowness issue with git completion

[Benjamin R. Haskell]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 3504 bytes --]

On Wed, 27 Apr 2011, Felipe Contreras wrote:

> On Wed, Apr 27, 2011 at 1:03 AM, Nikolai Weibull wrote:
>> On Tue, Apr 26, 2011 at 23:35, Felipe Contreras wrote:
>>> On Wed, Apr 27, 2011 at 12:13 AM, Mikael Magnusson wrote:
>>
>>>> It's only slow in repos as large as the linux one.
>>
>>> No. It's slow everywhere, it's *dead* slow on Linux. And BTW, 
>>> there's thousands of developers working on Linux (the kernel), I 
>>> guess we are not important.
>>
>> So perhaps you can improve it then.  Or one of the thousands of 
>> developers working on Linux (the kernel).  (I don’t believe that a 
>> very large fraction of them actually use Zsh,
>
> And now I see why.
>
>> though, or someone would have already provided a patch.)  Or perhaps 
>> you could patch the kernel to make it faster?
>
> It works fine in bash.
>
>> I’ve spent innumerable hours on making it correct.  I expect someone 
>> to pitch in a couple of hours to make it fast.
>
> Well, kernel and git developers have spent many more hours to make git 
> fast,  and this "correctness" is disrespecting that work because the 
> time it takes to execute the completion is longer than the command 
> itself.

Developing a slow solution around something highly optimized isn't 
"disrespecting" anything.  It's not taking advantage of that 
optimization, but that's hardly an act of "disrespect".


>> I find your attitude in this thread rather offensive.
>
> Oh, if I say the current implementation is dead slow and unusable, you
> find that offensive?

He presumably finds the snarky and/or holier-than-thou comments offensive:

"And now I see why"

"It works fine in bash"

"there's thousands of developers working on Linux (the kernel), I guess we are not important"

"Seriously, this is very annoying" [from your earlier message]

multiple dismissive "So?" responses [also in earlier messages]


> Maybe you should understand that criticism to code is not criticism to 
> you. But I will tell you what I do when somebody finds a fatal flaw in 
> my software; I fix it. If the person is offensive or cordial doesn't 
> really matter, the fatal flaw is there.

Many people do not consider cordiality to be an optional component of a 
request for volunteer effort.  The kernel community has a very different 
level of expected tact, apparently and expectedly.


> Now, how about you make a compromise between "correctness" and 
> usability? There's many parts of the code marked as TODO, so I guess 
> the code is not "perfectly correct". Add one more TODO, so in the 
> meantime 'git add' shows unchanged files, which would make the rest of 
> the commands as fast as they should be.

[see my other response -- gist: if it weren't broken the "context 
sensitivity" would be preferred, so the effort to rip it out perhaps isn't 
deemed to be worthwhile]


> I will tell you why I use completion; because I want to be more 
> productive. If completion makes me less productive, I will obviously 
> not use it... What's the point? I don't see why that is so hard to 
> understand. Note that this is the case also for small projects.
>
> Are you interested in fixing this use-case even if it means to make 
> some compromises in correctness or not?

I'll take a look at a better workaround tonight.  People who understand 
completion better than I do can continue on whichever path they prefer 
(whether the choice is "Worse is Better" vs "the Right Thing" OR 
cathedral vs bazaar is left as an exercise to the reader).

-- 
Best,
Ben

Re: Slowness issue with git completion

[gi1242+zsh]

On Wed, Apr 27, 2011 at 01:25:32AM +0300, Felipe Contreras wrote:

> Oh, if I say the current implementation is dead slow and unusable, you
> find that offensive? Maybe you should understand that criticism to
> code is not criticism to you.

My 2 cents: I've not written the git code, and I seldom use it. So
criticism to this code doesn't really affect me on any level. But I
found your tone hostile, and your comments rude.

Given that most people who work for zsh are volunteers, pissing them off
might not be the best way to go about things. Perhaps you might want to
take a look at

    http://www.catb.org/~esr/faqs/smart-questions.html

Specifically

    http://www.catb.org/~esr/faqs/smart-questions.html#courtesy
    http://www.catb.org/~esr/faqs/smart-questions.html#not_losing

GI

-- 
The 10 Commandments of DOS:
7. Know in thy heart that DOS shall always maintain backward
compatibility to the holy 2.0 version, blindly ignoring opportunities to
become compatible with things created in the latter half of this
century. But you can still run WordStar 1.0

Re: Slowness issue with git completion

[Bart Schaefer]

Having just watched the CentOS mailing list descend into a morass of
increasingly snarky complaints and increasingly hostile and defensive 
responses, I'd really prefer not to have that happen here.

Suggested improvements are welcome.

Implemented improvements are even more welcome.

Assuming that the lack of any particular improvement is due to anything
other than a lack of either man-hours or of the expertise to devote to
making it, is probably misguided.  Getting offended about that to the
point of saying "if you don't like it, go elsewhere" is also misguided.

With that out of the way:

Specifically with respect to completion functions, it's true that there
has been a tendency to take the concept of "context-sensitivity" to an
extreme.  Quite a few completion functions go to considerable lengths
to not just suggest possible correct answers but to avoid suggesting
any incorrect answers.  Often this introduces inefficiency (cf. the
recent change to _deb_packages, workers/28710).

As was suggested in the _deb_packages thread, a good place to start if
you aren't able to implement an efficiency improvement yourself is to
find the slow code path with the zprof module.  I see someone did that
back in workers/28682 but the paste2.org reference has expired.  I
suspect, though, that as with _deb_packages the answer has little to
do with the way the starting set of files is chosen and everything to
do with zsh's parameter manipulation syntax being too grotty for a
large number of complex string manipulations.

Also as someone noted elsewhere, zsh isn't very efficient at doing
array appends or slices.  Both complex string manipulations and lots
of array slicing/appending are done in __git_files_relative which is
called from _git_files.  Not only that, but it does string ops on the
path prefix on every pass around the "for file in $rawfiles" loop;
that at least could be abstracted.

I suspect that if someone undertook to rewrite __git_files_relative
wholly or partly in something like sed or awk, the whole thing would
be down to ~2 seconds even for a large number of files.  Heck, even
redoing the nested loops to do one multi-element array slice each
instead of many single-element splices might make a huge difference.

Re: Slowness issue with git completion

[Nikolai Weibull]

On Wed, Apr 27, 2011 at 00:25, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> On Wed, Apr 27, 2011 at 1:03 AM, Nikolai Weibull <now@bitwi.se> wrote:

>> I find your attitude in this thread rather offensive.

> Oh, if I say the current implementation is dead slow and unusable, you
> find that offensive?

No.  I find your attitude offensive, not your intended meaning.

> But I will tell you what I do when
> somebody finds a fatal flaw in my software; I fix it. If the person is
> offensive or cordial doesn't really matter, the fatal flaw is there.

Then you’re a better person than me.

> Now, how about you make a compromise between "correctness" and
> usability?

No.

> I will tell you why I use completion

I didn’t ask.

> Are you interested in fixing this use-case even if it means to make
> some compromises in correctness or not?

No, I’m not.

Re: Slowness issue with git completion

[Nikolai Weibull]

On Wed, Apr 27, 2011 at 06:19, Bart Schaefer <schaefer@brasslantern.com> wrote:

> Also as someone noted elsewhere, zsh isn't very efficient at doing
> array appends or slices.  Both complex string manipulations and lots
> of array slicing/appending are done in __git_files_relative which is
> called from _git_files.  Not only that, but it does string ops on the
> path prefix on every pass around the "for file in $rawfiles" loop;
> that at least could be abstracted.
>
> I suspect that if someone undertook to rewrite __git_files_relative
> wholly or partly in something like sed or awk, the whole thing would
> be down to ~2 seconds even for a large number of files.  Heck, even
> redoing the nested loops to do one multi-element array slice each
> instead of many single-element splices might make a huge difference.

I’ll try to find time to look into this.

Re: Slowness issue with git completion

[Benjamin R. Haskell]

On Tue, 26 Apr 2011, Benjamin R. Haskell wrote:
> On Wed, 27 Apr 2011, Felipe Contreras wrote:
>> Now, how about you make a compromise between "correctness" and 
>> usability? [...] Are you interested in fixing this use-case even if 
>> it means to make some compromises in correctness or not?
>
> I'll take a look at a better workaround tonight.  [...]

Aside from the segfault that I just reported, the following seems to be 
a far better out-of-tree workaround (i.e. in ~/.zshrc or thereabouts). 
Completion returns in less than a second in the kernel repo, grabbing 
correct-looking files and directories.  But the segfault seems to occur 
when successfully completing a file in any dir other than the root, 
rendering this useless for the moment.

__git_files () {
     # localize options as appropriate
     local compadd_opts opts tag description
     # ignore compadd-related options that __git_files passes along
     zparseopts -D -E -a compadd_opts V: J: 1 2 n f X: M: P: S: r: R: q F:
     # ignore git ls-files options that __git_files uses
     zparseopts -D -E -a opts -- -cached -deleted -modified -others -ignored -unmerged -killed x+: --exclude+:
     # __git_files expects a tag and a description
     tag=$1 description=$2 ; shift 2
     # but we ignore those and call __git_tree_files against the HEAD revision
     # with the current PREFIX
     __git_tree_files ${PREFIX:-.} HEAD
}

If moved in-tree, this function could be renamed to __git_files_shortcircuit,
and the real __git_files could be modified to use this function except when
$curcontext =~ *:git-add: (perhaps conditioned on some zstyle).

I tend to favor "correctness" over shortcuts like this[1].  But, I'm also of
the opinion that  "virtually-instantaneous, kindof-correct" is better than
"virtually-forever, absolutely-correct"[2].

Will take a look at the segfault again tomorrow if no one sees something 
obvious.

-- 
Best,
Ben

[1] http://www.zsh.org/mla/workers/2010/msg00297.html
[2] http://lists.pioto.org/pipermail/paludis-user/2008-December/000726.html

Re: Slowness issue with git completion

[Felipe Contreras]

On Wed, Apr 27, 2011 at 9:11 AM, Nikolai Weibull <now@bitwi.se> wrote:
>> Are you interested in fixing this use-case even if it means to make
>> some compromises in correctness or not?
>
> No, I’m not.

I am not used to zsh development, so is Nikolai's opinion shared by
the rest? Even if I provide a fix that make things slightly less
correct but usable, you wouldn't take the patch?

-- 
Felipe Contreras

Re: Slowness issue with git completion

[Frank Terbeck]

Felipe Contreras wrote:
> On Wed, Apr 27, 2011 at 9:11 AM, Nikolai Weibull <now@bitwi.se> wrote:
>>> Are you interested in fixing this use-case even if it means to make
>>> some compromises in correctness or not?
>>
>> No, I’m not.
>
> I am not used to zsh development, so is Nikolai's opinion shared by
> the rest? Even if I provide a fix that make things slightly less
> correct but usable, you wouldn't take the patch?

I for one, am very fond of the way completion is done (albeit, not its
performance in huge repositories).  It actually *saves* me a whole lot
of time, because - for example - "git add" only completes file names
that actually *need* to be staged.  So I would be firmly against
throwing out Nikolai's hard work.

As I was saying in another mail¹, I was also thinking about a scheme to
give the user the option to choose smart (and slow) and utterly-dumb
(and quick) file completion in various contexts via zstyle.  A boolean
style is very quick to look up and wouldn't impact performance. That
way, each user could have it his/her way, fine tuned for all git
sub-commands. That would be something everyone should be able to live
with.

Also, improving performance of the existing code would be highly
appreciated.

Regards, Frank

¹ <87ei4o7p0h.fsf@ft.bewatermyfriend.org>

Re: Slowness issue with git completion

[Felipe Contreras]

On Wed, Apr 27, 2011 at 11:47 AM, Frank Terbeck <ft@bewatermyfriend.org> wrote:
> Felipe Contreras wrote:
>> On Wed, Apr 27, 2011 at 9:11 AM, Nikolai Weibull <now@bitwi.se> wrote:
>>>> Are you interested in fixing this use-case even if it means to make
>>>> some compromises in correctness or not?
>>>
>>> No, I’m not.
>>
>> I am not used to zsh development, so is Nikolai's opinion shared by
>> the rest? Even if I provide a fix that make things slightly less
>> correct but usable, you wouldn't take the patch?
>
> I for one, am very fond of the way completion is done (albeit, not its
> performance in huge repositories).  It actually *saves* me a whole lot
> of time, because - for example - "git add" only completes file names
> that actually *need* to be staged.  So I would be firmly against
> throwing out Nikolai's hard work.
>
> As I was saying in another mail¹, I was also thinking about a scheme to
> give the user the option to choose smart (and slow) and utterly-dumb
> (and quick) file completion in various contexts via zstyle.  A boolean
> style is very quick to look up and wouldn't impact performance. That
> way, each user could have it his/her way, fine tuned for all git
> sub-commands. That would be something everyone should be able to live
> with.
>
> Also, improving performance of the existing code would be highly
> appreciated.

I believe the only way to achieve good performance is to write new
helper commands in git.git. I could do that, but first I want the
completion to be fast _now_, and only when the "correct" completion
performs in a reasonable way, then enable it.

Since I'm not seeing a huge amount of reasonable responses (usability
is not important), I'm thinking there's really no point in trying to
improve the zsh completion and I should use the bash one instead.

-- 
Felipe Contreras

Re: Slowness issue with git completion

[Nikolai Weibull]

On Wed, Apr 27, 2011 at 10:30, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> On Wed, Apr 27, 2011 at 9:11 AM, Nikolai Weibull <now@bitwi.se> wrote:
>> Felie Contreras wrote:
>>> Are you interested in fixing this use-case even if it means to make
>>> some compromises in correctness or not?
>>
>> No, I’m not.
>
> I am not used to zsh development, so is Nikolai's opinion shared by
> the rest?

I am primarily not interested in fixing it when you pose it to me in
the manner that you do.  You may not be aware of it, but your way of
expressing yourself is rather inflammatory.

There’s a big difference between writing

  Would you consider rewriting the code so that it would be a lot
faster, while still being mostly correct?

and what you wrote, which is a commandment hidden inside a question.

If we can find a solution that’s a lot quicker and still maintains
some of the nice features that we currently have, preferably correct
tags and descriptions, then I’m certainly interested.  There have,
however, not been any suggestions made in this area until Frank posted
his suggestion yesterday and perhaps Benjamin’s suggestion has some
merits to it as well (posted today).  Oliver also mentioned rewriting
it in the way that he did for the perforce completion (or was it
Subversion?).  I did, as I’ve already said, try rewriting it along
those lines, but it didn’t help.  I’m glad that we’re having this
discussion, because, believe it or not, I’m not happy with the delays
either.  I will, however, not be told what to do.

> Even if I provide a fix that make things slightly less
> correct but usable, you wouldn't take the patch?

You haven’t offered to do so until now, so how could we take a stance
on it?  What you’ve been saying so far is that /we/ should rewrite it
in a way that /you/ want it to work.

Re: Slowness issue with git completion

[Felipe Contreras]

On Wed, Apr 27, 2011 at 1:15 PM, Nikolai Weibull <now@bitwi.se> wrote:
> On Wed, Apr 27, 2011 at 10:30, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> On Wed, Apr 27, 2011 at 9:11 AM, Nikolai Weibull <now@bitwi.se> wrote:
>>> Felie Contreras wrote:
>>>> Are you interested in fixing this use-case even if it means to make
>>>> some compromises in correctness or not?
>>>
>>> No, I’m not.
>>
>> I am not used to zsh development, so is Nikolai's opinion shared by
>> the rest?
>
> I am primarily not interested in fixing it when you pose it to me in
> the manner that you do.  You may not be aware of it, but your way of
> expressing yourself is rather inflammatory.
>
> There’s a big difference between writing

I do realize it, I don't care. I'm not here to make you feel good
about yourself, I'm here to get something done. And I didn't command
anything, I asked a question. You are prejudging.

> If we can find a solution that’s a lot quicker and still maintains
> some of the nice features that we currently have, preferably correct
> tags and descriptions, then I’m certainly interested.

I don't think there's any right now, which why I am insisting on a
compromise. Feel free to prove me wrong.

> There have,
> however, not been any suggestions made in this area until Frank posted
> his suggestion yesterday and perhaps Benjamin’s suggestion has some
> merits to it as well (posted today).  Oliver also mentioned rewriting
> it in the way that he did for the perforce completion (or was it
> Subversion?).  I did, as I’ve already said, try rewriting it along
> those lines, but it didn’t help.  I’m glad that we’re having this
> discussion, because, believe it or not, I’m not happy with the delays
> either.  I will, however, not be told what to do.

I am explaining what I think should be done. If you agree or not is a
question, whether you want to do it or not is a different question.

>> Even if I provide a fix that make things slightly less
>> correct but usable, you wouldn't take the patch?
>
> You haven’t offered to do so until now, so how could we take a stance
> on it?  What you’ve been saying so far is that /we/ should rewrite it
> in a way that /you/ want it to work.

Again, you are imagining things. I always argued what I think should
be done. Who ends up doing it is irrelevant; or does my proposal
suddenly becomes good if I do it, but bad if you do it?

But you said:

> > Are you interested in fixing this use-case even if it means to make
> > some compromises in correctness or not?
>
> No, I’m not.

So why should I even try?

-- 
Felipe Contreras

Re: Slowness issue with git completion

[Nikolai Weibull]

On Wed, Apr 27, 2011 at 12:42, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> On Wed, Apr 27, 2011 at 1:15 PM, Nikolai Weibull <now@bitwi.se> wrote:
>> On Wed, Apr 27, 2011 at 10:30, Felipe Contreras
>> <felipe.contreras@gmail.com> wrote:
>>> On Wed, Apr 27, 2011 at 9:11 AM, Nikolai Weibull <now@bitwi.se> wrote:
>>>> Felie Contreras wrote:
>>>>> Are you interested in fixing this use-case even if it means to make
>>>>> some compromises in correctness or not?
>>>>
>>>> No, I’m not.
>>>
>>> I am not used to zsh development, so is Nikolai's opinion shared by
>>> the rest?
>>
>> I am primarily not interested in fixing it when you pose it to me in
>> the manner that you do.  You may not be aware of it, but your way of
>> expressing yourself is rather inflammatory.
>>
>> There’s a big difference between writing

> I do realize it, I don't care.

So my judgment of your attitude was correct.

> […]

I guess it’s time to end this meaningless line of discussion.

Re: Slowness issue with git completion

[Nikolai Weibull]

[-- Attachment #1: Type: text/plain, Size: 1738 bytes --]

On Wed, Apr 27, 2011 at 06:19, Bart Schaefer <schaefer@brasslantern.com> wrote:

> Also as someone noted elsewhere, zsh isn't very efficient at doing
> array appends or slices.  Both complex string manipulations and lots
> of array slicing/appending are done in __git_files_relative which is
> called from _git_files.  Not only that, but it does string ops on the
> path prefix on every pass around the "for file in $rawfiles" loop;
> that at least could be abstracted.
>
> I suspect that if someone undertook to rewrite __git_files_relative
> wholly or partly in something like sed or awk, the whole thing would
> be down to ~2 seconds even for a large number of files.  Heck, even
> redoing the nested loops to do one multi-element array slice each
> instead of many single-element splices might make a huge difference.

Let’s instead remove it.

The attached patch seems to speed up matching a lot, while maintaining
all functionality.  There might be cases that I haven’t considered.
Please review.

Another adjustment is that we could join the calls to git rev-parse
into one call:

gitcdupandprefix=${$(_call_program git-cdup-and-prefix git rev-parse
--show-cdup --show-prefix 2>/dev/null)/$'\n'}

to decrease the fork count as well, which makes a difference on
Cygwin.  This won’t be quite correct, however, as it won’t work for
prefixes with newlines in them.  Git-rev-parse sadly doesn’t take the
-z option.

Also, Bart, supplying -f to _multi_parts here doesn’t seem to have any
effect on match highlighting.  Directories are shown the same as
files.  This was something I asked about a long time ago that we were
going to look at at a later date.  Perhaps that date has come?

[-- Attachment #2: git.patch --]
[-- Type: application/octet-stream, Size: 3106 bytes --]

diff --git a/Completion/Unix/Command/_git b/Completion/Unix/Command/_git
index 4664cfa..6f71db3 100644
--- a/Completion/Unix/Command/_git
+++ b/Completion/Unix/Command/_git
@@ -5144,75 +5144,27 @@ __git_notes_refs () {
 
 # File Argument Types
 
-(( $+functions[__git_files_relative] )) ||
-__git_files_relative () {
-  local rawfiles files file f_parts prefix p_parts tmp
-
-  prefix=$(_call_program gitprefix git rev-parse --show-prefix 2>/dev/null)
-  __git_command_successful $pipestatus || return
-
-  # Empty prefix, no modifications
-  if (( $#prefix == 0 )); then
-    print $1
-    return
-  fi
-
-  rawfiles=(${(0)1})
-  files=()
-
-  # Now we assume that we've given "absolute" paths list with "root"
-  # being repository top directory.  $prefix is also "absolute" path.
-  for file in $rawfiles; do
-    # Collapse "/./" and "//", strip "/." and "/" from tail (I know,
-    # this is a bit paranoid).
-    f_parts=(${(s:/:)"${${${${file//\/\///}//\/.\///}%%/.}%%/}"})
-    p_parts=(${(s:/:)"${${${${prefix//\/\///}//\/.\///}%%/.}%%/}"})
-    tmp=()
-
-    # Strip common path prefix.
-    while (( $#f_parts > 0 )) && (( $#p_parts > 0 )) && [[ $f_parts[1] == $p_parts[1] ]]; do
-      f_parts[1]=()
-      p_parts[1]=()
-    done
-
-    # If prefix still not empty, ascend up.
-    while (( $#p_parts > 0 )); do
-      tmp+=..
-      p_parts[1]=()
-    done
-
-    # Add remaining path.
-    tmp+=($f_parts)
-
-    files+=${(j:/:)tmp}
-  done
-
-  print ${(pj:\0:)files}
-}
-
 (( $+functions[__git_files] )) ||
 __git_files () {
-  local compadd_opts opts tag description gitdir gitcdup files expl
+  local compadd_opts opts tag description gitcdup gitprefix files expl
 
   zparseopts -D -E -a compadd_opts V: J: 1 2 n f X: M: P: S: r: R: q F:
   zparseopts -D -E -a opts -- -cached -deleted -modified -others -ignored -unmerged -killed x+: --exclude+:
   tag=$1 description=$2; shift 2
 
-  gitdir=$(_call_program gitdir git rev-parse --git-dir 2>/dev/null)
+  gitcdup=$(_call_program gitcdup git rev-parse --show-cdup 2>/dev/null)
   __git_command_successful $pipestatus || return
 
-  gitcdup=$(_call_program gitcdup git rev-parse --show-cdup 2>/dev/null)
+  gitprefix=$(_call_program gitprefix git rev-parse --show-prefix 2>/dev/null)
   __git_command_successful $pipestatus || return
 
-  opts+='--exclude-per-directory=.gitignore'
-  [[ -f "$gitdir/info/exclude" ]] && opts+="--exclude-from=$gitdir/info/exclude"
+  # TODO: --directory should probably be added to $opts when --others is given.
 
-  files=$(_call_program files git ls-files -z --full-name $opts -- $gitcdup 2>/dev/null)
-  __git_command_successful $pipestatus || return
-  files=(${(0)"$(__git_files_relative $files)"})
+  local pref=$gitcdup$gitprefix$PREFIX
+  files=(${(0)"$(_call_program files git ls-files -z --exclude-standard $opts -- ${pref:+$pref/\*} 2>/dev/null)"})
   __git_command_successful $pipestatus || return
 
-  _wanted $tag expl $description _multi_parts $compadd_opts - / files
+  _wanted $tag expl $description _multi_parts -f $compadd_opts - / files
 }
 
 (( $+functions[__git_cached_files] )) ||

Re: Slowness issue with git completion

[Bart Schaefer]

On Wed, Apr 27, 2011 at 5:51 AM, Nikolai Weibull <now@bitwi.se> wrote:
> On Wed, Apr 27, 2011 at 06:19, Bart Schaefer <schaefer@brasslantern.com> wrote:
>
>> I suspect that if someone undertook to rewrite __git_files_relative
>
> Let’s instead remove it.

So much the better, now it's using only git operations.  Those who are
more active git-users than I should do the reviewing, though.

> Also, Bart, supplying -f to _multi_parts here doesn’t seem to have any
> effect on match highlighting.  Directories are shown the same as
> files.  This was something I asked about a long time ago that we were
> going to look at at a later date.  Perhaps that date has come?

See http://www.zsh.org/mla/workers//2010/msg00853.html (just so I can
be embarrassed again about the messed-up subject line).  I don't think
there's anything helpful to be done about _multi_parts, the coloring
is applied by the compadd builtin and the information it needs in
order to decide that something is a file is incompatible with the way
_multi_parts breaks up the strings.  Even if compadd could be called
the correct way, it will only color local stat-able files.

I hesitate to suggest this because it will be a lot of work for a
little shine, but you could try asserting a local value for ZLS_COLORS
in __git_files populated with pattern forms that produce the correct
coloring.

Re: Slowness issue with git completion

[Nikolai Weibull]

On Wed, Apr 27, 2011 at 16:55, Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Wed, Apr 27, 2011 at 5:51 AM, Nikolai Weibull <now@bitwi.se> wrote:

>> Also, Bart, supplying -f to _multi_parts here doesn’t seem to have any
>> effect on match highlighting.

> I don't think there's anything helpful to be done about _multi_parts

One, possibly slow, solution would be to use _files -g with a
generated pattern containing the literal matches.

Re: Slowness issue with git completion

[Simon Ruderich]

[-- Attachment #1: Type: text/plain, Size: 880 bytes --]

On Wed, Apr 27, 2011 at 02:51:05PM +0200, Nikolai Weibull wrote:
> [snip]
>
> Let’s instead remove it.
>
> The attached patch seems to speed up matching a lot, while maintaining
> all functionality.  There might be cases that I haven’t considered.
> Please review.

Seems to work fine for me (haven't tested it thoroughly though,
just normal usage).

However it looks like two __git_files_relative are still there,
in the functions:

    __git_changed-in-index_files
    __git_changed-in-working-tree_files

But I only noticed it when I tried this:

    git commit --am<tab>

    __git_changed-in-index_files:5: command not found: __git_files_relative
    __git_changed-in-working-tree_files:5: command not found: __git_files_relative

Regards,
Simon
-- 
+ privacy is necessary
+ using gnupg http://gnupg.org
+ public key id: 0x92FEFDB7E44C32F9

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: Slowness issue with git completion

[Simon Ruderich]

[-- Attachment #1: Type: text/plain, Size: 468 bytes --]

On Sat, Apr 30, 2011 at 04:37:18PM +0200, Simon Ruderich wrote:
> On Wed, Apr 27, 2011 at 02:51:05PM +0200, Nikolai Weibull wrote:
> [snip]
>
> However it looks like two __git_files_relative are still there,
> in the functions:
>
>     __git_changed-in-index_files
>     __git_changed-in-working-tree_files

Also occurs with git diff <tab>

Regards,
Simon
-- 
+ privacy is necessary
+ using gnupg http://gnupg.org
+ public key id: 0x92FEFDB7E44C32F9

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: Slowness issue with git completion

[Nikolai Weibull]

On Sat, Apr 30, 2011 at 16:37, Simon Ruderich <simon@ruderich.org> wrote:
> On Wed, Apr 27, 2011 at 02:51:05PM +0200, Nikolai Weibull wrote:
>> [snip]
>>
>> Let’s instead remove it.
>>
>> The attached patch seems to speed up matching a lot, while maintaining
>> all functionality.  There might be cases that I haven’t considered.
>> Please review.
>
> Seems to work fine for me (haven't tested it thoroughly though,
> just normal usage).
>
> However it looks like two __git_files_relative are still there,
> in the functions:
>
>    __git_changed-in-index_files
>    __git_changed-in-working-tree_files

Hm, weird.  I thought I did a proper search.  I’ll fix this tomorrow.

Re: Slowness issue with git completion

[Nikolai Weibull]

On Mon, May 2, 2011 at 11:59, Nikolai Weibull <now@bitwi.se> wrote:
> On Sat, Apr 30, 2011 at 16:37, Simon Ruderich <simon@ruderich.org> wrote:
>> However it looks like two __git_files_relative are still there,
>> in the functions:
>>
>>    __git_changed-in-index_files
>>    __git_changed-in-working-tree_files

> Hm, weird.  I thought I did a proper search.  I’ll fix this tomorrow.

It seems diff-index can’t be made to report results in the fashion
that we desire.  Suggestions welcome as to how we fix this.  It seems
__git_files_relative needs to stay.  Perhaps we can at least optimize
it in the ways Bart described?