* [PATCH] completion: complete refs after 'git restore -s'
@ 2020-09-25 14:25 Ákos Uzonyi
2020-09-25 17:30 ` Junio C Hamano
0 siblings, 1 reply; 3+ messages in thread
From: Ákos Uzonyi @ 2020-09-25 14:25 UTC (permalink / raw)
To: git; +Cc: Uzonyi Ákos, Nguyễn Thái Ngọc Duy,
Junio C Hamano
From: Uzonyi Ákos <uzonyi.akos@gmail.com>
Currently only the long version (--source=) supports completion.
Add completion support to the short (-s) option too.
Signed-off-by: Ákos Uzonyi <uzonyi.akos@gmail.com>
---
contrib/completion/git-completion.bash | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 8be4a0316e..50e6e82157 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2853,6 +2853,18 @@ _git_restore ()
--*)
__gitcomp_builtin restore
;;
+ *)
+ local prevword prevword="${words[cword-1]}"
+
+ case "$prevword" in
+ -s)
+ __git_complete_refs
+ return
+ ;;
+ *)
+ ;;
+ esac
+ ;;
esac
}
--
2.28.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] completion: complete refs after 'git restore -s'
2020-09-25 14:25 [PATCH] completion: complete refs after 'git restore -s' Ákos Uzonyi
@ 2020-09-25 17:30 ` Junio C Hamano
2020-09-25 19:59 ` Ákos Uzonyi
0 siblings, 1 reply; 3+ messages in thread
From: Junio C Hamano @ 2020-09-25 17:30 UTC (permalink / raw)
To: Ákos Uzonyi; +Cc: git, Nguyễn Thái Ngọc Duy
Ákos Uzonyi <uzonyi.akos@gmail.com> writes:
> From: Uzonyi Ákos <uzonyi.akos@gmail.com>
>
> Currently only the long version (--source=) supports completion.
>
> Add completion support to the short (-s) option too.
I am not too familiar with the completion library, but what makes
the "-s" option of restore so special? I've scanned the entire file
and did not find that many special cases for short options that have
their longer counterpart supported already.
I do not know if the "feature" this wants to bring in is a good
idea---we may want to try to be more systematic (e.g. perhaps it
involves teaching the parse-options subsystem about equivalence of
short and long options, so that we can reuse existing support for
the the long option "--source=<TAB>" to complete "-s <TAB>"), if we
were to do something like this. Singling out "-s" of "restore"
smells not quite right, as the approach would not scale well.
> Signed-off-by: Ákos Uzonyi <uzonyi.akos@gmail.com>
> ---
> contrib/completion/git-completion.bash | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 8be4a0316e..50e6e82157 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -2853,6 +2853,18 @@ _git_restore ()
> --*)
> __gitcomp_builtin restore
> ;;
> + *)
> + local prevword prevword="${words[cword-1]}"
Why duplicated prevword here? Did you mean
local prevword=${words[cword-1]}
instead?
> +
> + case "$prevword" in
> + -s)
> + __git_complete_refs
> + return
> + ;;
> + *)
> + ;;
> + esac
> + ;;
Wrong indentation. In this file, as can be seen on the line "*)"
you added at the top of this hunk, the case arms like "-s)" and "*)"
must align with "case" and "esac" in this file.
> esac
> }
Thanks.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] completion: complete refs after 'git restore -s'
2020-09-25 17:30 ` Junio C Hamano
@ 2020-09-25 19:59 ` Ákos Uzonyi
0 siblings, 0 replies; 3+ messages in thread
From: Ákos Uzonyi @ 2020-09-25 19:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Nguyễn Thái Ngọc Duy
Hi,
Thanks for your detailed review.
On Fri, 25 Sep 2020 at 19:30, Junio C Hamano <gitster@pobox.com> wrote:
> Ákos Uzonyi <uzonyi.akos@gmail.com> writes:
>
> > From: Uzonyi Ákos <uzonyi.akos@gmail.com>
> >
> > Currently only the long version (--source=) supports completion.
> >
> > Add completion support to the short (-s) option too.
>
> I am not too familiar with the completion library, but what makes
> the "-s" option of restore so special? I've scanned the entire file
> and did not find that many special cases for short options that have
> their longer counterpart supported already.
There are multiple commands already having this kind of short-long
option completion. The "-c" options of commit, switch and checkout
each have longer counterparts, and both the short and long versions
have completion support for their arguments.
> I do not know if the "feature" this wants to bring in is a good
> idea---we may want to try to be more systematic (e.g. perhaps it
> involves teaching the parse-options subsystem about equivalence of
> short and long options, so that we can reuse existing support for
> the the long option "--source=<TAB>" to complete "-s <TAB>"), if we
> were to do something like this. Singling out "-s" of "restore"
> smells not quite right, as the approach would not scale well.
I think these cases are not too frequent, so it doesn't seem to be a
big scaling problem.
> > Signed-off-by: Ákos Uzonyi <uzonyi.akos@gmail.com>
> > ---
> > contrib/completion/git-completion.bash | 12 ++++++++++++
> > 1 file changed, 12 insertions(+)
> >
> > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> > index 8be4a0316e..50e6e82157 100644
> > --- a/contrib/completion/git-completion.bash
> > +++ b/contrib/completion/git-completion.bash
> > @@ -2853,6 +2853,18 @@ _git_restore ()
> > --*)
> > __gitcomp_builtin restore
> > ;;
> > + *)
> > + local prevword prevword="${words[cword-1]}"
>
> Why duplicated prevword here? Did you mean
>
> local prevword=${words[cword-1]}
>
> instead?
Thanks, I'll fix it.
> > +
> > + case "$prevword" in
> > + -s)
> > + __git_complete_refs
> > + return
> > + ;;
> > + *)
> > + ;;
> > + esac
> > + ;;
>
> Wrong indentation. In this file, as can be seen on the line "*)"
> you added at the top of this hunk, the case arms like "-s)" and "*)"
> must align with "case" and "esac" in this file.
Thanks, I'll fix it.
By the way, I copied this piece of code from _git_switch (it's also
there in _git_checkout), so these problems have to be fixed there as
well.
Also, reading _git_commit it looks that we already have a "$prev"
variable, so I'll use that instead of "$prevword".
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-09-25 20:22 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-25 14:25 [PATCH] completion: complete refs after 'git restore -s' Ákos Uzonyi
2020-09-25 17:30 ` Junio C Hamano
2020-09-25 19:59 ` Ákos Uzonyi
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).