git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Todd Zullinger <tmz@pobox.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: "SZEDER Gábor" <szeder.dev@gmail.com>,
	"Keith Smiley" <k@keith.so>,
	git@vger.kernel.org, "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>,
	"Kevin Daudt" <me@ikke.info>
Subject: Re: [PATCH] Add shell completion for git remote rm
Date: Fri, 29 Dec 2017 19:52:22 -0500	[thread overview]
Message-ID: <20171230005222.GT3693@zaya.teonanacatl.net> (raw)
In-Reply-To: <87y3ll6s0e.fsf@evledraar.gmail.com>

Ævar Arnfjörð Bjarmason wrote:
>> I think adding 'rm' to completion definitely counts as advertisement.
>> It doesn't have much practical use, after all: typing 'rm' with
>> completion is actually one more keystroke than without (r<TAB>m vs. rm).
> 
> This is only one use of the completion interface, maybe you only use it
> like that, but not everyone does.
> 
> The completion interface has two uses, one is to actually save you
> typing, the other is subcommand discovery, and maybe someone who has a
> use neither you or I have thought of is about to point out a third.
> 
> I'll type "git $whatever $subcommand<TAB>" as *validation* that I've
> found the right command, not to complete it for me. This is a thing
> that's in my muscle memory for everything.

Is that meant to be in favor of including rm in the
completion results or against? :)

> Since I've been typing "git remote rm<TAB>" for a while (started before
> this deprecation happened) I've actually been meaning to submit
> completion for "rm" since it works, not knowing about Duy's patch until
> now.
> 
> Now, even if someone disagrees that we should have "rm" at all I think
> that in general we should not conflate two different things, one is
> whether:
> 
>     git remote <TAB>
> 
> shows both "rm" and "remove" in the list, and the other is whether:
> 
>     git remote rm<TAB>
> 
> Should yield:
> 
>     git remote rm<SPACE>
> 
> Or, as now happens:
> 
>     git remote rm<NOTHING AND ÆVAR THINKS IT'S BROKEN>
> 
> I can see why we'd, in general, we'd like to not advertise certain
> options for completion (due to deprecation, or just to avoid verbosity),
> but complete them once they're unambiguously typed out.
> 
> I don't know whether the bash completion interface supports making that
> distinction, but it would be useful.

It can be done, though I think that it's probably better to
subtly lead people to use 'git remote remove' going forward,
to keep things consistent.  I don't have a strong preference
for or against the rm -> remove change, but since that's
been done I think there's a benefit to keeping things
consistent in the UI.  And I think that should also apply to
not offering completion for commands/subcommands/options
which are only kept for backward compatibility.

Here's one way to make 'git remote rm <TAB>' work without
including it in the output of 'git remote <TAB>':

diff --git i/contrib/completion/git-completion.bash w/contrib/completion/git-completion.bash
index 3683c772c5..aa63f028ab 100644
--- i/contrib/completion/git-completion.bash
+++ w/contrib/completion/git-completion.bash
@@ -2668,7 +2668,9 @@ _git_remote ()
 		add rename remove set-head set-branches
 		get-url set-url show prune update
 		"
-	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")"
 	if [ -z "$subcommand" ]; then
 		case "$cur" in
 		--*)

Keeping 'git remote rm' working to avoid breaking scripts is
one thing, but having it in the completion code makes it
more likely that it will continue to be seen as a
recommended subcommand.

This leads to patches like this one, where it's presumed
that the lack of completion is simply an oversight or a bug.
Of course, the lack of completion hasn't caused everyone to
forget that 'remote rm' was changed to 'remote remove', so
that reasoning may be full of hot air (or worse). ;)

The current result of 'git remote rm <TAB>' isn't so great.
It's arguably worse to have it pretend that no subcommand
was given than to list the remotes.

$ git remote rm <TAB>
add            remove         set-head       update
get-url        rename         set-url
prune          set-branches   show

I think completing nothing or completing the remotes
(without offering rm in the subcommand list) would be
better, after looking at it a bit.

I don't know how to disable file completion, but I'm not
intimately familiar with the git completion script (thanks
to it working so damn well).  I'm guessing there's a way to
do that, if there's a strong desire to not complete the
remotes at all.

I don't think we should include rm in 'git remote <TAB>'
completion, but I don't care much either way what 'git
remote rm <TAB>' includes.  But it should be better than
including the other subcommands.

-- 
Todd
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
There, I've gone and soiled myself, are you happy now?!
    -- Stewie Griffin


  reply	other threads:[~2017-12-30  0:52 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-29  2:01 [PATCH] Add shell completion for git remote rm Keith Smiley
2017-12-29  3:29 ` Todd Zullinger
2017-12-29  4:19   ` Keith Smiley
2017-12-29 13:52     ` Todd Zullinger
2017-12-29 16:49       ` [PATCH] completion: restore 'remote rm' Keith Smiley
2017-12-29 22:48       ` [PATCH] Add shell completion for git remote rm SZEDER Gábor
2017-12-29 23:19         ` Keith Smiley
2017-12-29 23:20         ` Ævar Arnfjörð Bjarmason
2017-12-30  0:52           ` Todd Zullinger [this message]
2017-12-30 12:19             ` Ævar Arnfjörð Bjarmason
2017-12-31 20:44               ` Keith Smiley
2018-01-03 19:24               ` Junio C Hamano
2018-01-15 21:43                 ` Keith Smiley
2018-01-16 10:02                   ` Duy Nguyen
2018-01-16 18:57                     ` Junio C Hamano
2018-01-17  0:44                       ` Duy Nguyen
2018-01-17  6:17                         ` Kevin Daudt
2018-01-17  9:48                           ` Duy Nguyen
2018-07-16 13:12                             ` Keith Smiley
2018-01-17 18:20                         ` Junio C Hamano
2018-01-01 23:46     ` Duy Nguyen
2017-12-29  6:00 ` Kevin Daudt
2017-12-29  6:21   ` Keith Smiley

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171230005222.GT3693@zaya.teonanacatl.net \
    --to=tmz@pobox.com \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=k@keith.so \
    --cc=me@ikke.info \
    --cc=pclouds@gmail.com \
    --cc=szeder.dev@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).