reset: give better 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

However, we must also consider that some scripts may set
GIT_REFLOG_ACTION before calling reset, and we need to show
their reflog action (with our text appended). For example:

  rebase -i (squash): updating HEAD

On top of that, we also set the ORIG_HEAD reflog action
(even though it doesn't generally exist). In that case, the
reset argument is somewhat meaningless, as it has nothing to
do with what's in ORIG_HEAD.

This patch changes the reset reflog code to show:

  $GIT_REFLOG_ACTION: updating {HEAD,ORIG_HEAD}

as before, but only if GIT_REFLOG_ACTION is set. Otherwise,
show:

   reset: moving to $rev

for HEAD, and:

   reset: updating ORIG_HEAD

for ORIG_HEAD (this is still somewhat superfluous, since we
are in the ORIG_HEAD reflog, obviously, but at least we now
mention which command was used to update it).

While we're at it, we can clean up the code a bit:

 - Use strbufs to make the message.

 - Use the "rev" parameter instead of showing all options.
   This makes more sense, since it is the only thing
   impacting the writing of the ref.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

Author: peff

builtin/reset.c

@@ -33,25 +33,6 @@ static const char *reset_type_names[] = {
 	N_("mixed"), N_("soft"), N_("hard"), N_("merge"), N_("keep"), NULL
 };
 
-static char *args_to_str(const char **argv)
-{
-	char *buf = NULL;
-	unsigned long len, space = 0, nr = 0;
-
-	for (; *argv; argv++) {
-		len = strlen(*argv);
-		ALLOC_GROW(buf, nr + 1 + len, space);
-		if (nr)
-			buf[nr++] = ' ';
-		memcpy(buf + nr, *argv, len);
-		nr += len;
-	}
-	ALLOC_GROW(buf, nr + 1, space);
-	buf[nr] = '\0';
-
-	return buf;
-}
-
 static inline int is_merge(void)
 {
 	return !access(git_path("MERGE_HEAD"), F_OK);
@@ -215,14 +196,18 @@ static int read_from_tree(const char *prefix, const char **argv,
 	return update_index_refresh(index_fd, lock, refresh_flags);
 }
 
-static void prepend_reflog_action(const char *action, char *buf, size_t size)
+static void set_reflog_message(struct strbuf *sb, const char *action,
+			       const char *rev)
 {
-	const char *sep = ": ";
 	const char *rla = getenv("GIT_REFLOG_ACTION");
-	if (!rla)
-		rla = sep = "";
-	if (snprintf(buf, size, "%s%s%s", rla, sep, action) >= size)
-		warning(_("Reflog action message too long: %.*s..."), 50, buf);
+
+	strbuf_reset(sb);
+	if (rla)
+		strbuf_addf(sb, "%s: %s", rla, action);
+	else if (rev)
+		strbuf_addf(sb, "reset: moving to %s", rev);
+	else
+		strbuf_addf(sb, "reset: %s", action);
 }
 
 static void die_if_unmerged_cache(int reset_type)
@@ -241,7 +226,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 	unsigned char sha1[20], *orig = NULL, sha1_orig[20],
 				*old_orig = NULL, sha1_old_orig[20];
 	struct commit *commit;
-	char *reflog_action, msg[1024];
+	struct strbuf msg = STRBUF_INIT;
 	const struct option options[] = {
 		OPT__QUIET(&quiet, "be quiet, only report errors"),
 		OPT_SET_INT(0, "mixed", &reset_type,
@@ -261,8 +246,6 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 
 	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);
 
 	/*
 	 * Possible arguments are:
@@ -357,13 +340,13 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 		old_orig = sha1_old_orig;
 	if (!get_sha1("HEAD", sha1_orig)) {
 		orig = sha1_orig;
-		prepend_reflog_action("updating ORIG_HEAD", msg, sizeof(msg));
-		update_ref(msg, "ORIG_HEAD", orig, old_orig, 0, MSG_ON_ERR);
+		set_reflog_message(&msg, "updating ORIG_HEAD", NULL);
+		update_ref(msg.buf, "ORIG_HEAD", orig, old_orig, 0, MSG_ON_ERR);
 	}
 	else if (old_orig)
 		delete_ref("ORIG_HEAD", old_orig, 0);
-	prepend_reflog_action("updating HEAD", msg, sizeof(msg));
-	update_ref_status = update_ref(msg, "HEAD", sha1, orig, 0, MSG_ON_ERR);
+	set_reflog_message(&msg, "updating HEAD", rev);
+	update_ref_status = update_ref(msg.buf, "HEAD", sha1, orig, 0, MSG_ON_ERR);
 
 	switch (reset_type) {
 	case HARD:
@@ -380,7 +363,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 
 	remove_branch_state();
 
-	free(reflog_action);
+	strbuf_release(&msg);
 
 	return update_ref_status;
 }

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: moving to two
+		topic@{1} reset: moving to one
+		topic@{2} reset: moving to two
+		topic@{3} reset: moving to one
 		topic@{4} branch: Created from HEAD
 	EOF
 	git log -g --format="%gd %gs" topic >actual &&