Re: [PATCH 1/2] reflog: make the 'show' subcommand really the default

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Apr 19, 2010 at 05:46:02PM -0700, Junio C Hamano wrote:

> SZEDER Gábor <szeder@xxxxxxxxxx> writes:
> 
> > diff --git a/builtin/reflog.c b/builtin/reflog.c
> > index 64e45bd..d4d4409 100644
> > --- a/builtin/reflog.c
> > +++ b/builtin/reflog.c
> > @@ -712,6 +712,5 @@ int cmd_reflog(int argc, const char **argv, const char *prefix)
> >  	if (!strcmp(argv[1], "delete"))
> >  		return cmd_reflog_delete(argc - 1, argv + 1, prefix);
> >  
> > -	/* Not a recognized reflog command..*/
> > -	usage(reflog_usage);
> > +	return cmd_log_reflog(argc, argv, prefix);
> 
> I am not convinced that this is a good change.
> 
> It may be that show/expire/delete happens to be the _only_ subcommands
> today, but if we had this patch, the command will change the behaviour
> when we add a new subcommand (the name of that subcommand may happen to be
> also a refname).

I don't think it is that big a deal. Scripts should always use the
explicit "git reflog show <ref>" form, which will remain safe. "git
reflog <ref>" is handy for humans.

That being said, we tried this same experiment with "git stash [show]
<msg>" and ended up rejecting it. However, the main complaint with that
was the failure mode for typos. Typing "git stash sohw" would make a new
stash. In this case, typing "git reflog exipre" would get you:

  fatal: ambiguous argument 'exipre': unknown revision or path not in
  the working tree.
  Use '--' to separate paths from revisions

and you would repeat the command with the typo fixed, which is perhaps
not as bad. And unlike stash, "show" is really the dominant command (I
don't think I have ever manually used 'delete' or 'expire'), so it is
more likely to be right than not.

The current behavior is also weirdly inconsistent:

  git reflog ;# works, shows HEAD
  git reflog -p ;# works, shows HEAD with -p
  git reflog -p foo ;# works, shows foo with -p
  git reflog foo ;# does not work

I can see allowing the first two, but the fact that the third works and
the fourth doesn't seems odd to me.

So I don't think the patch is a huge mistake (at least not as huge as
the stash one).  However, I personally "git log -g [--oneline]" to be
much easier to remember to use and to type.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]