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>

Phillip Wood committed Dec 13, 2017 at 11:46 UTC 28d6daed4f119940ace31e523b3b272d3d153d04
4 files changed +61 -73
builtin/rebase--helper.c
+1 -12
@@ -9,17 +9,6 @@ static const char * const builtin_rebase_helper_usage[] = {
9 NULL
10 };
11
12 -static int git_rebase_helper_config(const char *k, const char *v, void *cb)
13 -{
14 - int status;
15 -
16 - status = git_sequencer_config(k, v, NULL);
17 - if (status)
18 - return status;
19 -
20 - return git_default_config(k, v, NULL);
21 -}
22 -
12 int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
13 {
14 struct replay_opts opts = REPLAY_OPTS_INIT;
@@ -50,7 +39,7 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
39 OPT_END()
40 };
41
53 - git_config(git_rebase_helper_config, NULL);
42 + sequencer_init_config(&opts);
43
44 opts.action = REPLAY_INTERACTIVE_REBASE;
45 opts.allow_ff = 1;
builtin/revert.c
+2 -13
@@ -31,17 +31,6 @@ static const char * const cherry_pick_usage[] = {
31 NULL
32 };
33
34 -static int common_config(const char *k, const char *v, void *cb)
35 -{
36 - int status;
37 -
38 - status = git_sequencer_config(k, v, NULL);
39 - if (status)
40 - return status;
41 -
42 - return git_default_config(k, v, NULL);
43 -}
44 -
34 static const char *action_name(const struct replay_opts *opts)
35 {
36 return opts->action == REPLAY_REVERT ? "revert" : "cherry-pick";
@@ -219,7 +208,7 @@ int cmd_revert(int argc, const char **argv, const char *prefix)
208 if (isatty(0))
209 opts.edit = 1;
210 opts.action = REPLAY_REVERT;
222 - git_config(common_config, NULL);
211 + sequencer_init_config(&opts);
212 res = run_sequencer(argc, argv, &opts);
213 if (res < 0)
214 die(_("revert failed"));
@@ -232,7 +221,7 @@ int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
221 int res;
222
223 opts.action = REPLAY_PICK;
235 - git_config(common_config, NULL);
224 + sequencer_init_config(&opts);
225 res = run_sequencer(argc, argv, &opts);
226 if (res < 0)
227 die(_("cherry-pick failed"));
sequencer.c
+48 -39
@@ -132,6 +132,51 @@ static GIT_PATH_FUNC(rebase_path_strategy, "rebase-merge/strategy")
132 static GIT_PATH_FUNC(rebase_path_strategy_opts, "rebase-merge/strategy_opts")
133 static GIT_PATH_FUNC(rebase_path_allow_rerere_autoupdate, "rebase-merge/allow_rerere_autoupdate")
134
135 +static int git_sequencer_config(const char *k, const char *v, void *cb)
136 +{
137 + struct replay_opts *opts = cb;
138 + int status;
139 +
140 + if (!strcmp(k, "commit.cleanup")) {
141 + const char *s;
142 +
143 + status = git_config_string(&s, k, v);
144 + if (status)
145 + return status;
146 +
147 + if (!strcmp(s, "verbatim"))
148 + opts->default_msg_cleanup = COMMIT_MSG_CLEANUP_NONE;
149 + else if (!strcmp(s, "whitespace"))
150 + opts->default_msg_cleanup = COMMIT_MSG_CLEANUP_SPACE;
151 + else if (!strcmp(s, "strip"))
152 + opts->default_msg_cleanup = COMMIT_MSG_CLEANUP_ALL;
153 + else if (!strcmp(s, "scissors"))
154 + opts->default_msg_cleanup = COMMIT_MSG_CLEANUP_SPACE;
155 + else
156 + warning(_("invalid commit message cleanup mode '%s'"),
157 + s);
158 +
159 + return status;
160 + }
161 +
162 + if (!strcmp(k, "commit.gpgsign")) {
163 + opts->gpg_sign = git_config_bool(k, v) ? "" : NULL;
164 + return 0;
165 + }
166 +
167 + status = git_gpg_config(k, v, NULL);
168 + if (status)
169 + return status;
170 +
171 + return git_diff_basic_config(k, v, NULL);
172 +}
173 +
174 +void sequencer_init_config(struct replay_opts *opts)
175 +{
176 + opts->default_msg_cleanup = COMMIT_MSG_CLEANUP_NONE;
177 + git_config(git_sequencer_config, opts);
178 +}
179 +
180 static inline int is_rebase_i(const struct replay_opts *opts)
181 {
182 return opts->action == REPLAY_INTERACTIVE_REBASE;
@@ -700,40 +745,6 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts,
745 return run_command(&cmd);
746 }
747
703 -static enum commit_msg_cleanup_mode default_msg_cleanup =
704 - COMMIT_MSG_CLEANUP_NONE;
705 -static char *default_gpg_sign;
706 -
707 -int git_sequencer_config(const char *k, const char *v, void *cb)
708 -{
709 - if (!strcmp(k, "commit.cleanup")) {
710 - int status;
711 - const char *s;
712 -
713 - status = git_config_string(&s, k, v);
714 - if (status)
715 - return status;
716 -
717 - if (!strcmp(s, "verbatim"))
718 - default_msg_cleanup = COMMIT_MSG_CLEANUP_NONE;
719 - else if (!strcmp(s, "whitespace"))
720 - default_msg_cleanup = COMMIT_MSG_CLEANUP_SPACE;
721 - else if (!strcmp(s, "strip"))
722 - default_msg_cleanup = COMMIT_MSG_CLEANUP_ALL;
723 - else if (!strcmp(s, "scissors"))
724 - default_msg_cleanup = COMMIT_MSG_CLEANUP_SPACE;
725 -
726 - return status;
727 - }
728 -
729 - if (!strcmp(k, "commit.gpgsign")) {
730 - default_gpg_sign = git_config_bool(k, v) ? "" : NULL;
731 - return 0;
732 - }
733 -
734 - return git_gpg_config(k, v, NULL);
735 -}
736 -
748 static int rest_is_empty(const struct strbuf *sb, int start)
749 {
750 int i, eol;
@@ -1039,7 +1050,6 @@ static int try_to_commit(struct strbuf *msg, const char *author,
1050 struct strbuf err = STRBUF_INIT;
1051 struct strbuf amend_msg = STRBUF_INIT;
1052 char *amend_author = NULL;
1042 - const char *gpg_sign;
1053 enum commit_msg_cleanup_mode cleanup;
1054 int res = 0;
1055
@@ -1072,7 +1082,8 @@ static int try_to_commit(struct strbuf *msg, const char *author,
1082 }
1083
1084 cleanup = (flags & CLEANUP_MSG) ? COMMIT_MSG_CLEANUP_ALL :
1075 - default_msg_cleanup;
1085 + opts->default_msg_cleanup;
1086 +
1087 if (cleanup != COMMIT_MSG_CLEANUP_NONE)
1088 strbuf_stripspace(msg, cleanup == COMMIT_MSG_CLEANUP_ALL);
1089 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,
1091 goto out;
1092 }
1093
1083 - gpg_sign = opts->gpg_sign ? opts->gpg_sign : default_gpg_sign;
1084 -
1094 if (write_cache_as_tree(tree.hash, 0, NULL)) {
1095 res = error(_("git write-tree failed to write a tree"));
1096 goto out;
@@ -1095,7 +1104,7 @@ static int try_to_commit(struct strbuf *msg, const char *author,
1104 }
1105
1106 if (commit_tree_extended(msg->buf, msg->len, tree.hash, parents,
1098 - oid->hash, author, gpg_sign, extra)) {
1107 + oid->hash, author, opts->gpg_sign, extra)) {
1108 res = error(_("failed to write commit object"));
1109 goto out;
1110 }
sequencer.h
+10 -9
@@ -11,6 +11,13 @@ enum replay_action {
11 REPLAY_INTERACTIVE_REBASE
12 };
13
14 +enum commit_msg_cleanup_mode {
15 + COMMIT_MSG_CLEANUP_SPACE,
16 + COMMIT_MSG_CLEANUP_NONE,
17 + COMMIT_MSG_CLEANUP_SCISSORS,
18 + COMMIT_MSG_CLEANUP_ALL
19 +};
20 +
21 struct replay_opts {
22 enum replay_action action;
23
@@ -29,6 +36,7 @@ struct replay_opts {
36 int mainline;
37
38 char *gpg_sign;
39 + enum commit_msg_cleanup_mode default_msg_cleanup;
40
41 /* Merge strategy */
42 char *strategy;
@@ -40,6 +48,8 @@ struct replay_opts {
48 };
49 #define REPLAY_OPTS_INIT { -1 }
50
51 +/* Call this to setup defaults before parsing command line options */
52 +void sequencer_init_config(struct replay_opts *opts);
53 int sequencer_pick_revisions(struct replay_opts *opts);
54 int sequencer_continue(struct replay_opts *opts);
55 int sequencer_rollback(struct replay_opts *opts);
@@ -57,15 +67,6 @@ extern const char sign_off_header[];
67
68 void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag);
69 void append_conflicts_hint(struct strbuf *msgbuf);
60 -int git_sequencer_config(const char *k, const char *v, void *cb);
61 -
62 -enum commit_msg_cleanup_mode {
63 - COMMIT_MSG_CLEANUP_SPACE,
64 - COMMIT_MSG_CLEANUP_NONE,
65 - COMMIT_MSG_CLEANUP_SCISSORS,
66 - COMMIT_MSG_CLEANUP_ALL
67 -};
68 -
70 int message_is_empty(const struct strbuf *sb,
71 enum commit_msg_cleanup_mode cleanup_mode);
72 int template_untouched(const struct strbuf *sb, const char *template_file,