Re: [GSoC] Some #leftoverbits for anyone looking for little projects

[Matheus Tavares <matheus.bernardino@usp.br>]

Hi, Ævar

> Give "rebase -i" some option so when you "reword" the patch is
> included in the message.
>
> I keep going to the shell because I have no idea what change I'm
> describing.

I have the same problem, so I wanted to try solving this. The patch
bellow creates a "rebase.verboseCommit" configuration that includes
a diff when rewording or squashing. I'd appreciate knowing your thoughts
on it.

As Christian wisely pointed out to me, though, we can also achieve this
behavior by setting "commit.verbose" to true. The only "downside" of it
is that users cannot choose to see the diff only when rebasing. Despite
of that, if we decide not to go with this patch, what do you think of
adding a "commit.verbose" entry at git-rebase's man page? 

diff --git a/Documentation/config/rebase.txt b/Documentation/config/rebase.txt
index d98e32d812..ae50b3e05d 100644
--- a/Documentation/config/rebase.txt
+++ b/Documentation/config/rebase.txt
@@ -62,3 +62,8 @@ rebase.rescheduleFailedExec::
 	Automatically reschedule `exec` commands that failed. This only makes
 	sense in interactive mode (or when an `--exec` option was provided).
 	This is the same as specifying the `--reschedule-failed-exec` option.
+
+rebase.verboseCommit::
+	When rewording or squashing commits, during an interactive rebase, show
+	the commits' diff to help describe the modifications they bring. False
+	by default.
diff --git a/sequencer.c b/sequencer.c
index f88a97fb10..1596fc4cd0 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -914,6 +914,7 @@ N_("you have staged changes in your working tree\n"
 #define CLEANUP_MSG (1<<3)
 #define VERIFY_MSG  (1<<4)
 #define CREATE_ROOT_COMMIT (1<<5)
+#define VERBOSE_COMMIT (1<<6)
 
 static int run_command_silent_on_success(struct child_process *cmd)
 {
@@ -1007,6 +1008,8 @@ static int run_git_commit(struct repository *r,
 		argv_array_push(&cmd.args, "-n");
 	if ((flags & AMEND_MSG))
 		argv_array_push(&cmd.args, "--amend");
+	if ((flags & VERBOSE_COMMIT))
+		argv_array_push(&cmd.args, "-v");
 	if (opts->gpg_sign)
 		argv_array_pushf(&cmd.args, "-S%s", opts->gpg_sign);
 	if (defmsg)
@@ -1782,7 +1785,7 @@ static int do_pick_commit(struct repository *r,
 	char *author = NULL;
 	struct commit_message msg = { NULL, NULL, NULL, NULL };
 	struct strbuf msgbuf = STRBUF_INIT;
-	int res, unborn = 0, allow;
+	int res, unborn = 0, allow, verbose_commit = 0;
 
 	if (opts->no_commit) {
 		/*
@@ -1843,6 +1846,9 @@ static int do_pick_commit(struct repository *r,
 		return error(_("cannot get commit message for %s"),
 			oid_to_hex(&commit->object.oid));
 
+	if (git_config_get_maybe_bool("rebase.verbosecommit", &verbose_commit) < 0)
+		warning("Invalid value for rebase.verboseCommit. Using 'false' instead.");
+
 	if (opts->allow_ff && !is_fixup(command) &&
 	    ((parent && oideq(&parent->object.oid, &head)) ||
 	     (!parent && unborn))) {
@@ -1853,6 +1859,8 @@ static int do_pick_commit(struct repository *r,
 		if (res || command != TODO_REWORD)
 			goto leave;
 		flags |= EDIT_MSG | AMEND_MSG | VERIFY_MSG;
+		if (verbose_commit)
+			flags |= VERBOSE_COMMIT;
 		msg_file = NULL;
 		goto fast_forward_edit;
 	}
@@ -1909,12 +1917,17 @@ static int do_pick_commit(struct repository *r,
 			author = get_author(msg.message);
 	}
 
-	if (command == TODO_REWORD)
+	if (command == TODO_REWORD) {
 		flags |= EDIT_MSG | VERIFY_MSG;
+		if (verbose_commit)
+			flags |= VERBOSE_COMMIT;
+	}
 	else if (is_fixup(command)) {
 		if (update_squash_messages(r, command, commit, opts))
 			return -1;
 		flags |= AMEND_MSG;
+		if (verbose_commit)
+			flags |= VERBOSE_COMMIT;
 		if (!final_fixup)
 			msg_file = rebase_path_squash_msg();
 		else if (file_exists(rebase_path_fixup_msg())) {
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 1723e1a858..9b410d31e2 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1477,4 +1477,60 @@ test_expect_success 'valid author header when author contains single quote' '
 	test_cmp expected actual
 '
 
+write_script "reword-and-check-for-diff" <<\EOF &&
+case "$1" in
+*/git-rebase-todo)
+	sed s/pick/reword/ "$1" > "$1.tmp"
+	mv -f "$1.tmp" "$1"
+	;;
+*)
+	grep '^diff --git' "$1" >has-diff
+	;;
+esac
+exit 0
+EOF
+
+test_expect_success 'rebase -i does not show diff by default when rewording' '
+	rebase_setup_and_clean no-verbose-commit-reword &&
+	test_set_editor "$PWD/reword-and-check-for-diff" &&
+	git rebase -i HEAD~1 &&
+	test_line_count = 0 has-diff
+'
+
+test_expect_success 'rebase -i respects rebase.verboseCommit when rewording' '
+	rebase_setup_and_clean verbose-commit-reword &&
+	test_config rebase.verboseCommit true &&
+	test_set_editor "$PWD/reword-and-check-for-diff" &&
+	git rebase -i HEAD~1 &&
+	test_line_count -gt 0 has-diff
+'
+
+write_script "squash-and-check-for-diff" <<\EOF &&
+case "$1" in
+*/git-rebase-todo)
+	sed "s/pick \([0-9a-f]*\) E/squash \1 E/" "$1" > "$1.tmp"
+	mv -f "$1.tmp" "$1"
+	;;
+*)
+	grep '^diff --git' "$1" >has-diff
+	;;
+esac
+exit 0
+EOF
+
+test_expect_success 'rebase -i does not show diff by default when squashing' '
+	rebase_setup_and_clean no-verbose-commit-squash &&
+	test_set_editor "$PWD/squash-and-check-for-diff" &&
+	git rebase -i HEAD~2 &&
+	test_line_count = 0 has-diff
+'
+
+test_expect_success 'rebase -i respects rebase.verboseCommit when squashing' '
+	rebase_setup_and_clean verbose-commit-squash &&
+	test_config rebase.verboseCommit true &&
+	test_set_editor "$PWD/squash-and-check-for-diff" &&
+	git rebase -i HEAD~2 &&
+	test_line_count -gt 0 has-diff
+'
+
 test_done
-- 
2.20.1