{"thread":{"id":"23524","subject":"[PATCH 2/2] reflog: remove 'show' from 'expire's usage string","startedAt":"2010-04-19T09:52:29Z","lastAt":"2010-04-22T14:40:59Z","messageCount":6,"participants":["SZEDER Gábor","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"139888","messageId":"1271670750-30297-1-git-send-email-szeder@ira.uka.de","threadId":"23524","inReplyTo":null,"subject":"[PATCH 1/2] reflog: make the 'show' subcommand really the default","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2010-04-19T09:52:29Z","receivedAt":"2010-04-19T09:52:29Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Since the introduction of the 'show' subcommand in cf39f54e (git\nreflog show, 2007-02-08), 'git reflog's documentation states that in\nthe absence of any subcommands 'show' is the default.  Although 'git\nreflog <log-options>' works as described, i.e. it defaults to 'show',\nthe command 'git reflog <ref>' errors out showing the usage instead of\nshowing the specified ref's reflog.  Fix this by treating an unknown\nsubcommand as a ref for 'show'.\n\nSigned-off-by: SZEDER Gábor <szeder@ira.uka.de>\n---\n builtin/reflog.c |    3 +--\n 1 files changed, 1 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex 64e45bd..d4d4409 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -712,6 +712,5 @@ int cmd_reflog(int argc, const char **argv, const char *prefix)\n \tif (!strcmp(argv[1], \"delete\"))\n \t\treturn cmd_reflog_delete(argc - 1, argv + 1, prefix);\n \n-\t/* Not a recognized reflog command..*/\n-\tusage(reflog_usage);\n+\treturn cmd_log_reflog(argc, argv, prefix);\n }\n-- \n1.7.1.rc1.43.gf0841\n"},{"id":"139887","messageId":"1271670750-30297-2-git-send-email-szeder@ira.uka.de","threadId":"23524","inReplyTo":"1271670750-30297-1-git-send-email-szeder@ira.uka.de","subject":"[PATCH 2/2] reflog: remove 'show' from 'expire's usage string","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2010-04-19T09:52:30Z","receivedAt":"2010-04-19T09:52:30Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Most of 'expire's options are not recognized by the 'show' subcommand,\nhence it errors out.\n\nSigned-off-by: SZEDER Gábor <szeder@ira.uka.de>\n---\n builtin/reflog.c |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex d4d4409..fe8b36d 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -13,7 +13,7 @@\n  */\n \n static const char reflog_expire_usage[] =\n-\"git reflog (show|expire) [--verbose] [--dry-run] [--stale-fix] [--expire=<time>] [--expire-unreachable=<time>] [--all] <refs>...\";\n+\"git reflog expire [--verbose] [--dry-run] [--stale-fix] [--expire=<time>] [--expire-unreachable=<time>] [--all] <refs>...\";\n static const char reflog_delete_usage[] =\n \"git reflog delete [--verbose] [--dry-run] [--rewrite] [--updateref] <refs>...\";\n \n-- \n1.7.1.rc1.43.gf0841\n"},{"id":"139946","messageId":"7veiiblzn9.fsf@alter.siamese.dyndns.org","threadId":"23524","inReplyTo":"1271670750-30297-1-git-send-email-szeder@ira.uka.de","subject":"Re: [PATCH 1/2] reflog: make the 'show' subcommand really the default","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-04-20T00:46:02Z","receivedAt":"2010-04-20T00:46:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor <szeder@ira.uka.de> writes:\n\n> diff --git a/builtin/reflog.c b/builtin/reflog.c\n> index 64e45bd..d4d4409 100644\n> --- a/builtin/reflog.c\n> +++ b/builtin/reflog.c\n> @@ -712,6 +712,5 @@ int cmd_reflog(int argc, const char **argv, const char *prefix)\n>  \tif (!strcmp(argv[1], \"delete\"))\n>  \t\treturn cmd_reflog_delete(argc - 1, argv + 1, prefix);\n>  \n> -\t/* Not a recognized reflog command..*/\n> -\tusage(reflog_usage);\n> +\treturn cmd_log_reflog(argc, argv, prefix);\n\nI am not convinced that this is a good change.\n\nIt may be that show/expire/delete happens to be the _only_ subcommands\ntoday, but if we had this patch, the command will change the behaviour\nwhen we add a new subcommand (the name of that subcommand may happen to be\nalso a refname).\n\nI'd rather see a fix for the documentation.\n\nYour [PATCH 2/2] looks very sane, though.\n\nThanks.\n"},{"id":"139964","messageId":"20100420113736.GA22907@coredump.intra.peff.net","threadId":"23524","inReplyTo":"7veiiblzn9.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/2] reflog: make the 'show' subcommand really the default","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-04-20T11:37:36Z","receivedAt":"2010-04-20T11:37:36Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Apr 19, 2010 at 05:46:02PM -0700, Junio C Hamano wrote:\n\n> SZEDER Gábor <szeder@ira.uka.de> writes:\n> \n> > diff --git a/builtin/reflog.c b/builtin/reflog.c\n> > index 64e45bd..d4d4409 100644\n> > --- a/builtin/reflog.c\n> > +++ b/builtin/reflog.c\n> > @@ -712,6 +712,5 @@ int cmd_reflog(int argc, const char **argv, const char *prefix)\n> >  \tif (!strcmp(argv[1], \"delete\"))\n> >  \t\treturn cmd_reflog_delete(argc - 1, argv + 1, prefix);\n> >  \n> > -\t/* Not a recognized reflog command..*/\n> > -\tusage(reflog_usage);\n> > +\treturn cmd_log_reflog(argc, argv, prefix);\n> \n> I am not convinced that this is a good change.\n> \n> It may be that show/expire/delete happens to be the _only_ subcommands\n> today, but if we had this patch, the command will change the behaviour\n> when we add a new subcommand (the name of that subcommand may happen to be\n> also a refname).\n\nI don't think it is that big a deal. Scripts should always use the\nexplicit \"git reflog show <ref>\" form, which will remain safe. \"git\nreflog <ref>\" is handy for humans.\n\nThat being said, we tried this same experiment with \"git stash [show]\n<msg>\" and ended up rejecting it. However, the main complaint with that\nwas the failure mode for typos. Typing \"git stash sohw\" would make a new\nstash. In this case, typing \"git reflog exipre\" would get you:\n\n  fatal: ambiguous argument 'exipre': unknown revision or path not in\n  the working tree.\n  Use '--' to separate paths from revisions\n\nand you would repeat the command with the typo fixed, which is perhaps\nnot as bad. And unlike stash, \"show\" is really the dominant command (I\ndon't think I have ever manually used 'delete' or 'expire'), so it is\nmore likely to be right than not.\n\nThe current behavior is also weirdly inconsistent:\n\n  git reflog ;# works, shows HEAD\n  git reflog -p ;# works, shows HEAD with -p\n  git reflog -p foo ;# works, shows foo with -p\n  git reflog foo ;# does not work\n\nI can see allowing the first two, but the fact that the third works and\nthe fourth doesn't seems odd to me.\n\nSo I don't think the patch is a huge mistake (at least not as huge as\nthe stash one).  However, I personally \"git log -g [--oneline]\" to be\nmuch easier to remember to use and to type.\n\n-Peff\n"},{"id":"140128","messageId":"20100422135731.GS4440@neumann","threadId":"23524","inReplyTo":"20100420113736.GA22907@coredump.intra.peff.net","subject":"Re: [PATCH 1/2] reflog: make the 'show' subcommand really the default","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2010-04-22T13:57:31Z","receivedAt":"2010-04-22T13:57:31Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Hi,\n\nOn Tue, Apr 20, 2010 at 07:37:36AM -0400, Jeff King wrote:\n> On Mon, Apr 19, 2010 at 05:46:02PM -0700, Junio C Hamano wrote:\n> \n> > I am not convinced that this is a good change.\n> > \n> > It may be that show/expire/delete happens to be the _only_ subcommands\n> > today, but if we had this patch, the command will change the behaviour\n> > when we add a new subcommand (the name of that subcommand may happen to be\n> > also a refname).\n> \n> I don't think it is that big a deal. Scripts should always use the\n> explicit \"git reflog show <ref>\" form, which will remain safe. \"git\n> reflog <ref>\" is handy for humans.\n> \n> That being said, we tried this same experiment with \"git stash [show]\n> <msg>\" and ended up rejecting it. However, the main complaint with that\n> was the failure mode for typos. Typing \"git stash sohw\" would make a new\n> stash.\n\nI think you meant \"git stash [save] <msg>\" here, right?\n\nhttp://thread.gmane.org/gmane.comp.version-control.git/63566/focus=63645\n\n> In this case, typing \"git reflog exipre\" would get you:\n> \n>   fatal: ambiguous argument 'exipre': unknown revision or path not in\n>   the working tree.\n>   Use '--' to separate paths from revisions\n> \n> and you would repeat the command with the typo fixed, which is perhaps\n> not as bad. And unlike stash, \"show\" is really the dominant command (I\n> don't think I have ever manually used 'delete' or 'expire'), so it is\n> more likely to be right than not.\n\nIf I understood Junio correctly, he is concerned about a different\ncase.\n\nWith my patch \"git reflog foo\" would show foo's reflog, assuming there\nis a branch named \"foo\".  This is what people would expect according\nto the documentation.  However, once we implement the subcommand\n\"foo\", \"git reflog foo\" will no longer print foo's reflog but instead\nwill perform whatever the subcommand \"foo\" is supposed to do by\ndefault, which might be difficult to recover from.\n\n> The current behavior is also weirdly inconsistent:\n> \n>   git reflog ;# works, shows HEAD\n>   git reflog -p ;# works, shows HEAD with -p\n>   git reflog -p foo ;# works, shows foo with -p\n>   git reflog foo ;# does not work\n> \n> I can see allowing the first two, but the fact that the third works and\n> the fourth doesn't seems odd to me.\n\nIndeed.  I tried to whip up a patch to fix the documentation to match\nthe current behaviour, but my first attempt failed because of these\ninconsistencies.\n\n> However, I personally \"git log -g [--oneline]\" to be\n> much easier to remember to use and to type.\n\nJust as sidenote:  I rarely use reflog, but then for me it's easier to\nremember to \"reflog\" than \"-g\".  But even then I never remember \"show\"\nfor the first time, that's why I wrote this patch.\n\n\nBest,\nGábor\n"},{"id":"140134","messageId":"20100422144059.GA28923@coredump.intra.peff.net","threadId":"23524","inReplyTo":"20100422135731.GS4440@neumann","subject":"Re: [PATCH 1/2] reflog: make the 'show' subcommand really the default","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-04-22T14:40:59Z","receivedAt":"2010-04-22T14:40:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Apr 22, 2010 at 03:57:31PM +0200, SZEDER Gábor wrote:\n\n> > That being said, we tried this same experiment with \"git stash [show]\n> > <msg>\" and ended up rejecting it. However, the main complaint with that\n> > was the failure mode for typos. Typing \"git stash sohw\" would make a new\n> > stash.\n> \n> I think you meant \"git stash [save] <msg>\" here, right?\n> \n> http://thread.gmane.org/gmane.comp.version-control.git/63566/focus=63645\n\nEr, yeah, that is what I meant. Sorry, thinko on my part.\n\n> > In this case, typing \"git reflog exipre\" would get you:\n> > \n> >   fatal: ambiguous argument 'exipre': unknown revision or path not in\n> >   the working tree.\n> >   Use '--' to separate paths from revisions\n> > \n> > and you would repeat the command with the typo fixed, which is perhaps\n> > not as bad. And unlike stash, \"show\" is really the dominant command (I\n> > don't think I have ever manually used 'delete' or 'expire'), so it is\n> > more likely to be right than not.\n> \n> If I understood Junio correctly, he is concerned about a different\n> case.\n> \n> With my patch \"git reflog foo\" would show foo's reflog, assuming there\n> is a branch named \"foo\".  This is what people would expect according\n> to the documentation.  However, once we implement the subcommand\n> \"foo\", \"git reflog foo\" will no longer print foo's reflog but instead\n> will perform whatever the subcommand \"foo\" is supposed to do by\n> default, which might be difficult to recover from.\n\nRight, what you quoted from me was not Junio's concern. But I argued\nagainst Junio's concern earlier in the mail. Basically, it is a corner\ncase. _If_ we add a new command to \"git reflog\", and _if_ that command\nis the same as the name of your ref, you must stop using the short form\nfor that ref, and instead use \"git reflog show $ref\".\n\nSo for things like scripts that don't know what $ref will be, and want\nto be on the safe side, they should continue to use the explicit \"git\nreflog show $ref\" form. But there is no reason that humans typing a\ncommand need to be bound by that restriction when it is unlikely to\nhappen (and even if it did, they would recover from it easily).\n\nPersonally, I don't care either way whether the documentation is updated\nto match the behavior of vice versa. But I just don't see what your\noriginal patch did as very harmful, and I can see how it would be\nhelpful (on the few occassions when I did use \"git reflog show\", I first\ntyped it forgetting the \"show\").\n\n-Peff\n"}]}