Re: Merge made by recursive?
- From
Jeff King <peff@peff.net>
- Date
- May 25, 2011, 21:25 UTC
- Message-ID
- <20110525212510.GA14214@sigill.intra.peff.net>
- In-Reply-To
- <20110525210254.GA29716@sigill.intra.peff.net>
On Wed, May 25, 2011 at 05:02:54PM -0400, Jeff King wrote:
Show 11 quoted lines
> On Wed, May 25, 2011 at 01:47:34PM -0700, Junio C Hamano wrote: > > > I am reluctant to do this (including the rewording of the end-user facing > > message) until we decide what to do with the reflog. Right now, I think no > > tool looks at the reflog, but contaminating the reflog with translatable > > messages mean that we will never be able to support "3 merges ago" just > > like we support "the previous branch". > > The reflog messages look like: > > merge $branch: Merge made by recursive.
While peeking in my reflog, I noticed some very confusing entries, which this patch addresses.
-- >8 -- Subject: [PATCH] reset: give more verbose reflog messages
The reset command creates its reflog entry from argv. However, it does so after having run parse_options, which means the only thing left in argv is any non-option arguments. Thus you would end up with confusing reflog entries like:
$ git reset --hard HEAD^
$ git reset --soft HEAD@{1}
$ git log -2 -g --oneline
8e46cad HEAD@{0}: HEAD@{1}: updating HEAD
1eb9486 HEAD@{1}: HEAD^: updating HEADThis patch sets up the reflog before argv is munged, so you get the command name and any other options, like:
8e46cad HEAD@{0}: reset --soft HEAD@{1}: updating HEAD
1eb9486 HEAD@{1}: reset --hard HEAD^: updating HEADSigned-off-by: Jeff King <peff@peff.net> --- I am not sure if this was the original intent of the code or not; I had to update a test vector which codified it. Any options like "--hard" or "--soft" are actually superfluous to the ref update (not to mention something like "-q"). So another option would be to just take what's left after parsing options and putting "reset" in front of it, like:
8e46cad HEAD@{0}: reset: HEAD^: updating HEADwhich is a little more readable. Though if we are going to change it, I think my preference would actually be:
8e46cad HEAD@{0}: reset: moving to HEAD^which reads better. The "updating HEAD" is just pointless. Of course we're updating HEAD; we're in the HEAD reflog and we're running reset!
However, if GIT_REFLOG_ACTION is already set (by a script calling us), then we won't say "reset". So for example, I have entries in my reflog like:
944af8c HEAD@{311}: rebase -i (squash): updating HEADSo maybe it makes sense to leave those ones as-is, and adjust only the case where GIT_REFLOG_ACTION is unset.
builtin/reset.c | 5 +++-- t/t1412-reflog-loop.sh | 8 ++++---- 2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c index 98bca04..77103fb 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -259,11 +259,12 @@ int cmd_reset(int argc, const char **argv, const char *prefix) git_config(git_default_config, NULL); - argc = parse_options(argc, argv, prefix, options, git_reset_usage, - PARSE_OPT_KEEP_DASHDASH); reflog_action = args_to_str(argv); setenv("GIT_REFLOG_ACTION", reflog_action, 0); + argc = parse_options(argc, argv, prefix, options, git_reset_usage, + PARSE_OPT_KEEP_DASHDASH); + /* * Possible arguments are: * diff --git a/t/t1412-reflog-loop.sh b/t/t1412-reflog-loop.sh index 7f519e5..a92875f 100755 --- a/t/t1412-reflog-loop.sh +++ b/t/t1412-reflog-loop.sh @@ -21,10 +21,10 @@ test_expect_success 'setup reflog with alternating commits' ' test_expect_success 'reflog shows all entries' ' cat >expect <<-\EOF - topic@{0} two: updating HEAD - topic@{1} one: updating HEAD - topic@{2} two: updating HEAD - topic@{3} one: updating HEAD + topic@{0} reset two: updating HEAD + topic@{1} reset one: updating HEAD + topic@{2} reset two: updating HEAD + topic@{3} reset one: updating HEAD topic@{4} branch: Created from HEAD EOF git log -g --format="%gd %gs" topic >actual &&
-- 1.7.4.5.34.g0787f