Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bash/zsh completion: reimplement and decrease runtime by factor 1863 #960

Merged
merged 1 commit into from
Feb 23, 2020

Conversation

rohieb
Copy link

@rohieb rohieb commented Oct 17, 2019

Fixes: #795 ("tab completion is really slow")

When run on a Git repository with a large amount of files, like the linux repository, __tig_complete_file() can take a very long time to complete. This is unfortunate because when accidentally pressing Tab in such a repo, bash gets stuck until the completion function has finished, and does not even allow the user to cancel the operation with Ctrl-C.

In contrast, the current git completion does not have these problems, and since tig's command line parameters are mostly parameters to git-log or git-diff, we can use git's native completion for those cases instead. This also has the advantage that we do not need to care about updating the tig completion when new parameters are added to git-log or git-diff.

I have tested this only in bash, not on zsh, so it would be nice to get some input from zsh users.

For comparison, here is an exemplary runtime measurement of the old and the new completion in bash, showing an improvement of factor 1863. Admittedly, this was on a fairly loaded system (a build server), but still then the new completion runs in unnoticable time. I'm also getting similar results on an idle system in the same repo with runtime improvements of about factor 1000.

linux (master) $ echo $BASH_VERSION
5.0.3(1)-release

linux (master) $ git describe
v5.4-rc3-38-gbc88f85c6c09

linux (master) $ uptime
 16:45:52 up 36 days,  3:33, 224 users,  load average: 24.17, 38.87, 31.21

# The old completion:
linux (master) $ . /usr/share/bash-completion/completions/tig
linux (master) $ time COMP_WORDS=("tig log") COMP_CWORD=2 _tig

real    2m1.145s
user    1m40.379s
sys     0m1.347s

# The new completion:
linux (master) $ . ../tig/contrib/tig-completion.bash
linux (master) $ time COMP_WORDS=("tig log") COMP_CWORD=2 __git_wrap_tig

real    0m0.127s
user    0m0.085s
sys     0m0.024s

With this change, almost nothing of the old completion remains, so change the copyright header accordingly. I'm also now adding a GPL-2.0-or-later dedication, which is the same license as most other code in this repository, and which, I presume, was also the author's intent since the first incarnation of this file.

@markrian
Copy link

markrian commented Jan 14, 2020

This is great, @rohieb, thank you!

EDIT: Disregard all of the below! I forgot I first attempted to source these completions a different way, before just copying directly to /usr/share/bash-completion/completions/tig. After cleaning that up, this just worked! 🎉

Previous version of comment

After copying this to /usr/share/bash-completion/completions/tig, I'm finding that __git_complete isn't defined in a given terminal session until I've tried to complete a git command. That is, I need to attempt to complete a git command before I attempt to complete a tig command; otherwise nothing happens.

An example from a new terminal session:

-bash: __git_complete: command not found

$ complete | grep "\(git\|tig\)"
complete -o bashdefault -o default -F _fzf_path_completion git

$ git show mas<tab><tab> ^C # this is where I try to complete a git command

$ complete | grep "\(git\|tig\)"
complete -o bashdefault -o default -o nospace -F __git_wrap__gitk_main gitk
complete -o bashdefault -o default -o nospace -F _fzf_path_completion git

I have no idea how bash completions are wired up, but it seems weird to me that things that are hooked up with complete change after trying to complete commands!

Is there something I'm missing, here?

@koutcher
Copy link
Collaborator

@hohieb, could you add the missing tig -C argument and reflog command (completion should be identical to tig log) and rebase ?

@rohieb
Copy link
Author

rohieb commented Feb 15, 2020

@koutcher ah, thanks for the note. -C was a bit tricky because of the missing space before the path, until I found the compgen -P option. Learned something along the way! :D

I also updated the install comments and made a few other minor style and typo fixes.

@koutcher
Copy link
Collaborator

Great. One problem though, If you do tig -C<Tab>, make a completion and then <Space><Tab>, bash goes into infinite loop. The cause seems to be the continue that was added line 57 to -*) ;;.
Also, tig reflog does not take the same argument as git reflog. It is more like tig log, so I believe we should have:

diff --git a/contrib/tig-completion.bash b/contrib/tig-completion.bash
index e7756d9..355b15f 100755
--- a/contrib/tig-completion.bash
+++ b/contrib/tig-completion.bash
@@ -73,6 +73,9 @@ _tig() {
 		refs|status|stash)
 			COMPREPLY=( $(compgen -W "$__tig_options" -- "$cur") )
 			;;
+		reflog)
+			__git_complete_command log
+			;;
 		"")
 			__git_complete_command log
 			__gitcompappend "$(compgen -W "$__tig_options $__tig_commands" -- "$cur")"

@rohieb
Copy link
Author

rohieb commented Feb 16, 2020

If you do tig -C<Tab>, make a completion and then <Space><Tab>, bash goes into infinite loop.

Mh, of course I forgot to test this case at all, or else I would've noticed 😳… Should be fixed now.

Also, tig reflog does not take the same argument as git reflog. It is more like tig log

Thanks for the clarification, I now did it as per your suggestion.

Fixes: jonas#795 ("tab completion is really slow")

When run on a Git repository with a large amount of files, like the
linux repository, __tig_complete_file() can take a very long time to
complete. This is unfortunate because when accidentally pressing Tab in
such a repo, bash gets stuck until the completion function has finished,
and does not even allow the user to cancel the operation with Ctrl-C.

In contrast, the current git completion does not have these problems,
and since tig's command line parameters are mostly parameters to git-log
or git-diff, we can use git's native completion for those cases instead.
This also has the advantage that we do not need to care about updating
the tig completion when new parameters are added to git-log or git-diff.

I have tested this only in bash, not in zsh.

For comparison, here is an exemplary runtime measurement of the old and
the new completion in bash, showing an improvement of factor 1863.
Admittedly, this was on a fairly loaded system (a build server), but
still then the new completion runs in unnoticable time. I'm also getting
similar results on an idle system in the same repo with runtime
improvements of about factor 1000.

    linux (master) $ echo $BASH_VERSION
    5.0.3(1)-release

    linux (master) $ git describe
    v5.4-rc3-38-gbc88f85c6c09

    linux (master) $ uptime
     16:45:52 up 36 days,  3:33, 224 users,  load average: 24.17, 38.87, 31.21

    # The new completion:
    linux (master) $ . ../tig/contrib/tig-completion.bash
    linux (master) $ time COMP_WORDS=("tig log") COMP_CWORD=2 __git_wrap_tig

    real    0m0.127s
    user    0m0.085s
    sys     0m0.024s

    # The old completion:
    linux (master) $ . /usr/share/bash-completion/completions/tig
    linux (master) $ time COMP_WORDS=("tig log") COMP_CWORD=2 _tig

    real    2m1.145s
    user    1m40.379s
    sys     0m1.347s

With this change, almost nothing of the old completion remains, so
change the copyright header accordingly. I'm also now adding a
GPL-2.0-or-later dedication, which is the same license as most other
code in this repository; and which, I presume, was also the author's
intent since the first incarnation of this file.

While at it, fix some typos in old comments, and update installation
instructions.

Signed-off-by: Roland Hieber <rhi@pengutronix.de>
@fjarlq
Copy link

fjarlq commented Oct 29, 2020

According to @spazm in this comprehensive stackoverflow answer, this pull request created a regression in zsh completion:

https://stackoverflow.com/a/64164918/393684

@spazm writes:

[In zsh] Activating completion on tig breaks completion for git.

  • If tig is activated after git, then tig completion works and git completion is broken.
  • If tig completion is activated before git, then they are both broken.

[..]

Solution:
TODO: File issue with tig. This is a regression with the new completion script as implemented in #960

@fjarlq
Copy link

fjarlq commented Oct 29, 2020

@rohieb
Copy link
Author

rohieb commented Oct 29, 2020

@fjarlq it might be related to #940 too

@felipec
Copy link
Contributor

felipec commented Nov 3, 2020

@fjarlq My guess is that this commit might fix the issue.

@felipec
Copy link
Contributor

felipec commented Nov 3, 2020

Actually, no, the issue is that bashcompinit was loaded at the wrong time.

I fixed that here.

But also, there's a bunch of other issues. I'm sending a pull request.

@felipec felipec mentioned this pull request Nov 3, 2020
@darcyparker
Copy link
Contributor

@markrian FYI: In #960 (comment), the part labeled "Previous version of comment" has some details that could be explained by some of my notes in #1011 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tab completion is really slow
6 participants