Re: [PATCH] Add shell completion for git remote rm
- From
- Keith Smiley <k@keith.so>
- Date
- Dec 31, 2017, 20:44 UTC
- Message-ID
- <20171231204426.GA30674@bryant.local>
- In-Reply-To
- <87vago76i8.fsf@evledraar.gmail.com>
I'm definitely happy to update this patch for now to just complete the remote names, and not add rm to the list of subcommand completions if we're all ok with that!
-- Keith Smiley
On 12/30, Ævar Arnfjörð Bjarmason wrote:
Show 146 quoted lines
> >On Sat, Dec 30 2017, Todd Zullinger jotted: > >> Æ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? :) > >For. > >>> 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. > >We changed it in the past, we can always change it again, it's never too >late to fix the UI. > >Now whether we *should* change/fix this particular thing is another >matter. I'm just pointing out that we shouldn't fall into the trap of >thinking that git's UI is an established platform that can't be changed. > >The vast majority of people who'll ever use git will most likely start >using a version that we're going to release many years into the future. > >I'm reminded of the story about the guy who decided makefiles must have >tabs, who didn't want to change it because he already had some dozens of >users. > >> And I think that should also apply to >> not offering completion for commands/subcommands/options >> which are only kept for backward compatibility. > >Yeah I think it makes sense to at some point stop completing things if >we're going to remove stuff, if we decide to remove it. > >> 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 >> --*) > >Neat! > >> 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 > >Although that's a bug that has nothing to do with remove/rm, because you >also get: > > $ git remote blahblah <TAB> > $ git rebase doesntexist <TAB> > >etc. showing you valid subcommands, when perhaps we should show >"warning: no such subcommand `blahblah`/`doesntexist`!" instead. > >> 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.