Re: [PATCH] Add shell completion for git remote rm

From

Duy Nguyen <pclouds@gmail.com>

Date

Jan 17, 2018, 00:44 UTC

Message-ID

<20180117004419.GA14263@duynguyen.dek-tpc.internal>

In-Reply-To

<xmqqh8rlvdgh.fsf@gitster.mtv.corp.google.com>

On Tue, Jan 16, 2018 at 10:57:34AM -0800, Junio C Hamano wrote:

> Duy Nguyen <pclouds@gmail.com> writes:
> 
> > On Tue, Jan 16, 2018 at 4:43 AM, Keith Smiley <k@keith.so> wrote:
> >> So it sounds like either we should deprecate rm, or I should update the patch to the suggested method where we just complete remotes, but not rm in the list of completions.
> >
> > I vote for deprecation. I could send a patch to start warning to
> > switch from 'rm' to 'remove'. Then perhaps we can delete it after two
> > releases. It's not classified as plumbing should we don't have worry
> > too much about scripts relying on 'remote rm'.
> 
> I do not know about "two releases" part (which amounts to merely
> 20-24 weeks), but looking at "git remote -h" output and seeing that
> we do spell out "rename" (instead of saying "mv" or something cryptic),
> it probably makes sense to remove "rm" some time in the future.
> 
> I actually do think "rm" is _already_ deprecated.  
> 
> "git remote --help" does not mention it in its synopsis section and
> it merely has ", rm" after "remove" as if an afterthought.

> I am not sure if it is worth being more explicit, perhaps like this?

-- 8< --

diff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt
index 577b969c1b..0a544703e6 100644
--- a/Documentation/git-remote.txt
+++ b/Documentation/git-remote.txt
@@ -90,7 +90,6 @@ In case <old> and <new> are the same, and <old> is a file under
 the configuration file format.
 
 'remove'::
-'rm'::
 
 Remove the remote named <name>. All remote-tracking branches and
 configuration settings for the remote are removed.
diff --git a/builtin/remote.c b/builtin/remote.c
index d95bf904c3..774ef6931e 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -1609,7 +1609,10 @@ int cmd_remote(int argc, const char **argv, const char *prefix)
 		result = add(argc, argv);
 	else if (!strcmp(argv[0], "rename"))
 		result = mv(argc, argv);
-	else if (!strcmp(argv[0], "rm") || !strcmp(argv[0], "remove"))
+	else if (!strcmp(argv[0], "rm")) {
+		warning(_("'rm' is a deprecated synonym. Use 'remove' instead."));
+		result = rm(argc, argv);
+	} else if (!strcmp(argv[0], "remove"))
 		result = rm(argc, argv);
 	else if (!strcmp(argv[0], "set-head"))
 		result = set_head(argc, argv);
-- 8< --

PS. This also makes me thing about supporting subcommand aliases, so
that people can add back 'git remote rm' if they like (or something
like 'git r rm' when they alias 'remote' as well). Which is probably a
good thing to do. Long command names are fine when you have completion
support, they are a pain to type otherwise.

--
Duy