On Wed, Jan 17, 2018 at 07:44:19AM +0700, Duy Nguyen wrote:
> 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.
>
Couldn't they just create an alias like git rrm then, if they use it so
often that it becomes an issue? Having another layer of aliases does
create a lot of complexity.
> --
> Duy
