On Wed, May 25, 2011 at 05:02:54PM -0400, Jeff King wrote:
> 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 HEAD
This 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 HEAD
Signed-off-by: Jeff King <peff@xxxxxxxx>
---
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 HEAD
which 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 HEAD
So 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
--
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