| Message ID | 20190612085606.12144-1-pclouds@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] completion: do not cache if --git-completion-helper fails | expand |
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes: > "git <cmd> --git-completion-helper" could fail if the command checks for > a repo before parse_options(). If the result is cached, later on when > the user moves to a worktree with repo, tab completion will still fail. > > Avoid this by detecting errors and not cache the completion output. We > can try again and hopefully succeed next time (e.g. when a repo is > found). > > Of course if --git-completion-helper fails permanently because of other > reasons (*), this will slow down completion. But I don't see any better > option to handle that case. > > (*) one of those cases is if __gitcomp_builtin is called on a command > that does not support --git-completion-helper. And we do have a > generic call > > __git_complete_common "$command" > > but this case is protected with __git_support_parseopt_helper so we're > good. > > Reported-by: Felipe Contreras <felipe.contreras@gmail.com> > Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> > --- > v2 simplifies the code. $nocache in v1 was overkill. Nicely done. Thanks, all. Will queue. > > contrib/completion/git-completion.bash | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash > index 9f71bcde96..8c6b610a24 100644 > --- a/contrib/completion/git-completion.bash > +++ b/contrib/completion/git-completion.bash > @@ -400,7 +400,8 @@ __gitcomp_builtin () > if [ -z "$options" ]; then > # leading and trailing spaces are significant to make > # option removal work correctly. > - options=" $incl $(__git ${cmd/_/ } --git-completion-helper) " > + options=" $incl $(__git ${cmd/_/ } --git-completion-helper) " || return > + > for i in $excl; do > options="${options/ $i / }" > done
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 9f71bcde96..8c6b610a24 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -400,7 +400,8 @@ __gitcomp_builtin () if [ -z "$options" ]; then # leading and trailing spaces are significant to make # option removal work correctly. - options=" $incl $(__git ${cmd/_/ } --git-completion-helper) " + options=" $incl $(__git ${cmd/_/ } --git-completion-helper) " || return + for i in $excl; do options="${options/ $i / }" done
"git <cmd> --git-completion-helper" could fail if the command checks for a repo before parse_options(). If the result is cached, later on when the user moves to a worktree with repo, tab completion will still fail. Avoid this by detecting errors and not cache the completion output. We can try again and hopefully succeed next time (e.g. when a repo is found). Of course if --git-completion-helper fails permanently because of other reasons (*), this will slow down completion. But I don't see any better option to handle that case. (*) one of those cases is if __gitcomp_builtin is called on a command that does not support --git-completion-helper. And we do have a generic call __git_complete_common "$command" but this case is protected with __git_support_parseopt_helper so we're good. Reported-by: Felipe Contreras <felipe.contreras@gmail.com> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> --- v2 simplifies the code. $nocache in v1 was overkill. contrib/completion/git-completion.bash | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)