On Tue, Jan 16, 2018 at 10:57:34AM -0800, Junio C Hamano wrote:
> Duy Nguyen <pclouds@xxxxxxxxx> writes:
>
> > On Tue, Jan 16, 2018 at 4:43 AM, Keith Smiley <k@xxxxxxxx> 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.
It's actually my bad. I should have replaced 'rm' with 'remove' in
git-remote.txt in e17dba8fe1 (remote: prefer subcommand name 'remove'
to 'rm' - 2012-09-06)
> I am not sure if it is worth being more explicit, perhaps like this?
Looks good. If we want to be more careful, we can follow up with
something even more annoying like this before removing 'rm'
-- 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
