git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH] Add support for 'git remote rm' in Bash completion script
@ 2019-02-09  5:24 Sergey Zolotarev
  2019-02-09  6:27 ` Todd Zullinger
  0 siblings, 1 reply; 4+ messages in thread
From: Sergey Zolotarev @ 2019-02-09  5:24 UTC (permalink / raw)
  To: git; +Cc: Sergey Zolotarev

---
 contrib/completion/git-completion.bash | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 499e56f83d..fa25d689e2 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2334,7 +2334,7 @@ _git_remote ()
 {
 	local subcommands="
 		add rename remove set-head set-branches
-		get-url set-url show prune update
+		get-url set-url show prune rm update
 		"
 	local subcommand="$(__git_find_on_cmdline "$subcommands")"
 	if [ -z "$subcommand" ]; then
@@ -2379,6 +2379,9 @@ _git_remote ()
 	prune,--*)
 		__gitcomp_builtin remote_prune
 		;;
+	rm,--*)
+		__gitcomp_builtin remote_rm
+		;;
 	*)
 		__gitcomp_nl "$(__git_remotes)"
 		;;
-- 
2.20.1.windows.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] Add support for 'git remote rm' in Bash completion script
  2019-02-09  5:24 [PATCH] Add support for 'git remote rm' in Bash completion script Sergey Zolotarev
@ 2019-02-09  6:27 ` Todd Zullinger
  2019-02-09 16:47   ` Keith Smiley
  0 siblings, 1 reply; 4+ messages in thread
From: Todd Zullinger @ 2019-02-09  6:27 UTC (permalink / raw)
  To: Sergey Zolotarev; +Cc: git, Keith Smiley

Hi Sergey,

There was a previous discussion of this in December 2017,
which might be useful:

https://public-inbox.org/git/01020160a0004473-277c3d7c-4e3b-4c50-9d44-4a106f37f1d9-000000@eu-west-1.amazonses.com/

It didn't end with a patch applied, but it's likely worth
reading to help you make a case for a similar patch.

One of the points in that thread is that the rm subcommand
is not shown in completion intentionally, as the preferred
subcommand is remove.  But it should be possible to offer
completion of the remotes after a user types rm, which I
imagine is the more helpful part of the completion.

Also, you'll want to add a signoff to the patch if/when you
resend it (refer to Documentation/SubmittingPatches, if you
haven't already).

Sergey Zolotarev wrote:
> ---
>  contrib/completion/git-completion.bash | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 499e56f83d..fa25d689e2 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -2334,7 +2334,7 @@ _git_remote ()
>  {
>  	local subcommands="
>  		add rename remove set-head set-branches
> -		get-url set-url show prune update
> +		get-url set-url show prune rm update
>  		"
>  	local subcommand="$(__git_find_on_cmdline "$subcommands")"

So instead of this change you could adjust the subcommand
line, something like:

-	local subcommand="$(__git_find_on_cmdline "$subcommands")"
+	# Don't advertise rm by including it in subcommands, but complete
+	# remotes if it is used.
+	local subcommand="$(__git_find_on_cmdline "$subcommands rm")"

I haven't test that, but the code looks like it hasn't
changed since the last time we talked about this -- when I
did test the suggestion :).

>  	if [ -z "$subcommand" ]; then
> @@ -2379,6 +2379,9 @@ _git_remote ()
>  	prune,--*)
>  		__gitcomp_builtin remote_prune
>  		;;
> +	rm,--*)
> +		__gitcomp_builtin remote_rm
> +		;;
>  	*)
>  		__gitcomp_nl "$(__git_remotes)"
>  		;;

I don't think you need this chunk, do you?  I think that's
only useful for completing options to the subcommand, which
'git remote rm' lacks.

I believe you can simply skip it and let the case fall
through to the last item which simply completes the
available remotes, just as 'git remote remove' does.

Hope that helps,

-- 
Todd

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Add support for 'git remote rm' in Bash completion script
  2019-02-09  6:27 ` Todd Zullinger
@ 2019-02-09 16:47   ` Keith Smiley
  2019-02-10  5:58     ` Todd Zullinger
  0 siblings, 1 reply; 4+ messages in thread
From: Keith Smiley @ 2019-02-09 16:47 UTC (permalink / raw)
  To: Todd Zullinger; +Cc: Sergey Zolotarev, git

It would be great if we could land this. Non of the other solutions since I proposed my patch have happened, so in the meantime this would be nice to have.


--
Keith Smiley

> On Feb 8, 2019, at 22:27, Todd Zullinger <tmz@pobox.com> wrote:
> 
> Hi Sergey,
> 
> There was a previous discussion of this in December 2017,
> which might be useful:
> 
> https://public-inbox.org/git/01020160a0004473-277c3d7c-4e3b-4c50-9d44-4a106f37f1d9-000000@eu-west-1.amazonses.com/
> 
> It didn't end with a patch applied, but it's likely worth
> reading to help you make a case for a similar patch.
> 
> One of the points in that thread is that the rm subcommand
> is not shown in completion intentionally, as the preferred
> subcommand is remove.  But it should be possible to offer
> completion of the remotes after a user types rm, which I
> imagine is the more helpful part of the completion.
> 
> Also, you'll want to add a signoff to the patch if/when you
> resend it (refer to Documentation/SubmittingPatches, if you
> haven't already).
> 
> Sergey Zolotarev wrote:
>> ---
>> contrib/completion/git-completion.bash | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>> 
>> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
>> index 499e56f83d..fa25d689e2 100644
>> --- a/contrib/completion/git-completion.bash
>> +++ b/contrib/completion/git-completion.bash
>> @@ -2334,7 +2334,7 @@ _git_remote ()
>> {
>>    local subcommands="
>>        add rename remove set-head set-branches
>> -        get-url set-url show prune update
>> +        get-url set-url show prune rm update
>>        "
>>    local subcommand="$(__git_find_on_cmdline "$subcommands")"
> 
> So instead of this change you could adjust the subcommand
> line, something like:
> 
> -    local subcommand="$(__git_find_on_cmdline "$subcommands")"
> +    # Don't advertise rm by including it in subcommands, but complete
> +    # remotes if it is used.
> +    local subcommand="$(__git_find_on_cmdline "$subcommands rm")"
> 
> I haven't test that, but the code looks like it hasn't
> changed since the last time we talked about this -- when I
> did test the suggestion :).
> 
>>    if [ -z "$subcommand" ]; then
>> @@ -2379,6 +2379,9 @@ _git_remote ()
>>    prune,--*)
>>        __gitcomp_builtin remote_prune
>>        ;;
>> +    rm,--*)
>> +        __gitcomp_builtin remote_rm
>> +        ;;
>>    *)
>>        __gitcomp_nl "$(__git_remotes)"
>>        ;;
> 
> I don't think you need this chunk, do you?  I think that's
> only useful for completing options to the subcommand, which
> 'git remote rm' lacks.
> 
> I believe you can simply skip it and let the case fall
> through to the last item which simply completes the
> available remotes, just as 'git remote remove' does.
> 
> Hope that helps,
> 
> -- 
> Todd


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Add support for 'git remote rm' in Bash completion script
  2019-02-09 16:47   ` Keith Smiley
@ 2019-02-10  5:58     ` Todd Zullinger
  0 siblings, 0 replies; 4+ messages in thread
From: Todd Zullinger @ 2019-02-10  5:58 UTC (permalink / raw)
  To: Keith Smiley
  Cc: Sergey Zolotarev, Nguyễn Thái Ngọc Duy,
	Junio C Hamano, git

Hi,

Keith Smiley wrote:
>> On Feb 8, 2019, at 22:27, Todd Zullinger <tmz@pobox.com> wrote:
>> 
>> Hi Sergey,
>> 
>> There was a previous discussion of this in December 2017,
>> which might be useful:
>> 
>> https://public-inbox.org/git/01020160a0004473-277c3d7c-4e3b-4c50-9d44-4a106f37f1d9-000000@eu-west-1.amazonses.com/
>> 
>> It didn't end with a patch applied, but it's likely worth
>> reading to help you make a case for a similar patch.
>> 
>> One of the points in that thread is that the rm subcommand
>> is not shown in completion intentionally, as the preferred
>> subcommand is remove.  But it should be possible to offer
>> completion of the remotes after a user types rm, which I
>> imagine is the more helpful part of the completion.
>> 
>> Also, you'll want to add a signoff to the patch if/when you
>> resend it (refer to Documentation/SubmittingPatches, if you
>> haven't already).
>> 
>> Sergey Zolotarev wrote:
>>> ---
>>> contrib/completion/git-completion.bash | 5 ++++-
>>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>> 
>>> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
>>> index 499e56f83d..fa25d689e2 100644
>>> --- a/contrib/completion/git-completion.bash
>>> +++ b/contrib/completion/git-completion.bash
>>> @@ -2334,7 +2334,7 @@ _git_remote ()
>>> {
>>>    local subcommands="
>>>        add rename remove set-head set-branches
>>> -        get-url set-url show prune update
>>> +        get-url set-url show prune rm update
>>>        "
>>>    local subcommand="$(__git_find_on_cmdline "$subcommands")"
>> 
>> So instead of this change you could adjust the subcommand
>> line, something like:
>> 
>> -    local subcommand="$(__git_find_on_cmdline "$subcommands")"
>> +    # Don't advertise rm by including it in subcommands, but complete
>> +    # remotes if it is used.
>> +    local subcommand="$(__git_find_on_cmdline "$subcommands rm")"
>> 
>> I haven't test that, but the code looks like it hasn't
>> changed since the last time we talked about this -- when I
>> did test the suggestion :).
>> 
>>>    if [ -z "$subcommand" ]; then
>>> @@ -2379,6 +2379,9 @@ _git_remote ()
>>>    prune,--*)
>>>        __gitcomp_builtin remote_prune
>>>        ;;
>>> +    rm,--*)
>>> +        __gitcomp_builtin remote_rm
>>> +        ;;
>>>    *)
>>>        __gitcomp_nl "$(__git_remotes)"
>>>        ;;
>> 
>> I don't think you need this chunk, do you?  I think that's
>> only useful for completing options to the subcommand, which
>> 'git remote rm' lacks.
>> 
>> I believe you can simply skip it and let the case fall
>> through to the last item which simply completes the
>> available remotes, just as 'git remote remove' does.
>> 
>> Hope that helps,
>> 
> It would be great if we could land this. Non of the other
> solutions since I proposed my patch have happened, so in
> the meantime this would be nice to have.

Yeah, it seems like we should either complete the remotes
for 'git remote rm <tab>' or follow up with the removal of
the rm subcommand as Duy and Junio fleshed out in the thread
from July.  If the command were to be removed, doing it
right after the 2.22 cycle begins would be as good a time as
any.

If the command remains, here's a suggestion for how it might
be submitted (perhaps with more details in the commit to
justify why we're completing arguments to a deprecated
subcommand?).

It's really not an itch of mine, so I'm just tossing this
out in case someone that wants it more will push it forward.

---- >8 ----
Subject: [PATCH] completion: complete remotes for 'remote rm'

The remote 'rm' subcommand was removed from the completed subcommands
list in e17dba8fe1 ("remote: prefer subcommand name 'remove' to 'rm'",
2012-09-06).  While we don't advertise it, we can still complete the
list of remotes for users who type 'git remote rm <tab>' as a courtesy.

Reported-by: Keith Smiley <k@keith.so>
Reported-by: Sergey Zolotarev <szolot4rev@gmail.com>
Helped-by: Todd Zullinger <tmz@pobox.com>
Signed-off-by: <whomever polishes as needed>
---
 contrib/completion/git-completion.bash | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 499e56f83d..5d0f8a2077 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2336,7 +2336,9 @@ _git_remote ()
 		add rename remove set-head set-branches
 		get-url set-url show prune update
 		"
-	local subcommand="$(__git_find_on_cmdline "$subcommands")"
+	# We don't advertise rm by including it in subcommands, but if it is
+	# used we want to complete remotes.
+	local subcommand="$(__git_find_on_cmdline "$subcommands rm")"
 	if [ -z "$subcommand" ]; then
 		case "$cur" in
 		--*)

-- 
Todd

^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2019-02-10  5:58 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-09  5:24 [PATCH] Add support for 'git remote rm' in Bash completion script Sergey Zolotarev
2019-02-09  6:27 ` Todd Zullinger
2019-02-09 16:47   ` Keith Smiley
2019-02-10  5:58     ` Todd Zullinger

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).