sequencer: improve config handling

The previous config handling relied on global variables, called
git_default_config() even when the key had already been handled by
git_sequencer_config() and did not initialize the diff configuration
variables. Improve this by: i) loading the default values for message
cleanup and gpg signing of commits into struct replay_opts;
ii) restructuring the code to return immediately once a key is
handled; and iii) calling git_diff_basic_config(). Note that
unfortunately it is not possible to return early if the key is handled
by git_gpg_config() as it does not indicate to the caller if the key
has been handled or not.

The sequencer should probably have been calling
git_diff_basic_config() before as it creates a patch when there are
conflicts. The shell version uses 'diff-tree' to create the patch so
calling git_diff_basic_config() should match that. Although 'git
commit' calls git_diff_ui_config() I don't think the output of
print_commit_summary() is affected by anything that is loaded by that
as print_commit_summary() always turns on rename detection so would
ignore the value in the user's configuration anyway. The other values
loaded by git_diff_ui_config() are about the formatting of patches so
are not relevant to print_commit_summary().

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

1 files changed, 48 insertions, 39 deletions

diff --git a/sequencer.c b/sequencer.c
index 99452a0e89..7051b20b76 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -132,6 +132,51 @@ static GIT_PATH_FUNC(rebase_path_strategy, "rebase-merge/strategy")
 static GIT_PATH_FUNC(rebase_path_strategy_opts, "rebase-merge/strategy_opts")
 static GIT_PATH_FUNC(rebase_path_allow_rerere_autoupdate, "rebase-merge/allow_rerere_autoupdate")
 
+static int git_sequencer_config(const char *k, const char *v, void *cb)
+{
+	struct replay_opts *opts = cb;
+	int status;
+
+	if (!strcmp(k, "commit.cleanup")) {
+		const char *s;
+
+		status = git_config_string(&s, k, v);
+		if (status)
+			return status;
+
+		if (!strcmp(s, "verbatim"))
+			opts->default_msg_cleanup = COMMIT_MSG_CLEANUP_NONE;
+		else if (!strcmp(s, "whitespace"))
+			opts->default_msg_cleanup = COMMIT_MSG_CLEANUP_SPACE;
+		else if (!strcmp(s, "strip"))
+			opts->default_msg_cleanup = COMMIT_MSG_CLEANUP_ALL;
+		else if (!strcmp(s, "scissors"))
+			opts->default_msg_cleanup = COMMIT_MSG_CLEANUP_SPACE;
+		else
+			warning(_("invalid commit message cleanup mode '%s'"),
+				  s);
+
+		return status;
+	}
+
+	if (!strcmp(k, "commit.gpgsign")) {
+		opts->gpg_sign = git_config_bool(k, v) ? "" : NULL;
+		return 0;
+	}
+
+	status = git_gpg_config(k, v, NULL);
+	if (status)
+		return status;
+
+	return git_diff_basic_config(k, v, NULL);
+}
+
+void sequencer_init_config(struct replay_opts *opts)
+{
+	opts->default_msg_cleanup = COMMIT_MSG_CLEANUP_NONE;
+	git_config(git_sequencer_config, opts);
+}
+
 static inline int is_rebase_i(const struct replay_opts *opts)
 {
 	return opts->action == REPLAY_INTERACTIVE_REBASE;
@@ -700,40 +745,6 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts,
 	return run_command(&cmd);
 }
 
-static enum commit_msg_cleanup_mode default_msg_cleanup =
-						COMMIT_MSG_CLEANUP_NONE;
-static char *default_gpg_sign;
-
-int git_sequencer_config(const char *k, const char *v, void *cb)
-{
-	if (!strcmp(k, "commit.cleanup")) {
-		int status;
-		const char *s;
-
-		status = git_config_string(&s, k, v);
-		if (status)
-			return status;
-
-		if (!strcmp(s, "verbatim"))
-			default_msg_cleanup = COMMIT_MSG_CLEANUP_NONE;
-		else if (!strcmp(s, "whitespace"))
-			default_msg_cleanup = COMMIT_MSG_CLEANUP_SPACE;
-		else if (!strcmp(s, "strip"))
-			default_msg_cleanup = COMMIT_MSG_CLEANUP_ALL;
-		else if (!strcmp(s, "scissors"))
-			default_msg_cleanup = COMMIT_MSG_CLEANUP_SPACE;
-
-		return status;
-	}
-
-	if (!strcmp(k, "commit.gpgsign")) {
-		default_gpg_sign = git_config_bool(k, v) ? "" : NULL;
-		return 0;
-	}
-
-	return git_gpg_config(k, v, NULL);
-}
-
 static int rest_is_empty(const struct strbuf *sb, int start)
 {
 	int i, eol;
@@ -1039,7 +1050,6 @@ static int try_to_commit(struct strbuf *msg, const char *author,
 	struct strbuf err = STRBUF_INIT;
 	struct strbuf amend_msg = STRBUF_INIT;
 	char *amend_author = NULL;
-	const char *gpg_sign;
 	enum commit_msg_cleanup_mode cleanup;
 	int res = 0;
 
@@ -1072,7 +1082,8 @@ static int try_to_commit(struct strbuf *msg, const char *author,
 	}
 
 	cleanup = (flags & CLEANUP_MSG) ? COMMIT_MSG_CLEANUP_ALL :
-					  default_msg_cleanup;
+					  opts->default_msg_cleanup;
+
 	if (cleanup != COMMIT_MSG_CLEANUP_NONE)
 		strbuf_stripspace(msg, cleanup == COMMIT_MSG_CLEANUP_ALL);
 	if (!opts->allow_empty_message && message_is_empty(msg, cleanup)) {
@@ -1080,8 +1091,6 @@ static int try_to_commit(struct strbuf *msg, const char *author,
 		goto out;
 	}
 
-	gpg_sign = opts->gpg_sign ? opts->gpg_sign : default_gpg_sign;
-
 	if (write_cache_as_tree(tree.hash, 0, NULL)) {
 		res = error(_("git write-tree failed to write a tree"));
 		goto out;
@@ -1095,7 +1104,7 @@ static int try_to_commit(struct strbuf *msg, const char *author,
 	}
 
 	if (commit_tree_extended(msg->buf, msg->len, tree.hash, parents,
-				 oid->hash, author, gpg_sign, extra)) {
+				 oid->hash, author, opts->gpg_sign, extra)) {
 		res = error(_("failed to write commit object"));
 		goto out;
 	}