Skip to content

Commit 28d6dae

Browse files
phillipwoodgitster
phillipwood
authored and
gitster
committed
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 parent db9476b commit 28d6dae

File tree

4 files changed

+61
-73
lines changed

4 files changed

+61
-73
lines changed

‎builtin/rebase--helper.c‎

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,17 +9,6 @@ static const char * const builtin_rebase_helper_usage[] = {
99
NULL
1010
};
1111

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-
2312
int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
2413
{
2514
struct replay_opts opts = REPLAY_OPTS_INIT;
@@ -50,7 +39,7 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
5039
OPT_END()
5140
};
5241

53-
git_config(git_rebase_helper_config, NULL);
42+
sequencer_init_config(&opts);
5443

5544
opts.action = REPLAY_INTERACTIVE_REBASE;
5645
opts.allow_ff = 1;

‎builtin/revert.c‎

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -31,17 +31,6 @@ static const char * const cherry_pick_usage[] = {
3131
NULL
3232
};
3333

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-
4534
static const char *action_name(const struct replay_opts *opts)
4635
{
4736
return opts->action == REPLAY_REVERT ? "revert" : "cherry-pick";
@@ -219,7 +208,7 @@ int cmd_revert(int argc, const char **argv, const char *prefix)
219208
if (isatty(0))
220209
opts.edit = 1;
221210
opts.action = REPLAY_REVERT;
222-
git_config(common_config, NULL);
211+
sequencer_init_config(&opts);
223212
res = run_sequencer(argc, argv, &opts);
224213
if (res < 0)
225214
die(_("revert failed"));
@@ -232,7 +221,7 @@ int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
232221
int res;
233222

234223
opts.action = REPLAY_PICK;
235-
git_config(common_config, NULL);
224+
sequencer_init_config(&opts);
236225
res = run_sequencer(argc, argv, &opts);
237226
if (res < 0)
238227
die(_("cherry-pick failed"));

‎sequencer.c‎

Lines changed: 48 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,51 @@ static GIT_PATH_FUNC(rebase_path_strategy, "rebase-merge/strategy")
132132
static GIT_PATH_FUNC(rebase_path_strategy_opts, "rebase-merge/strategy_opts")
133133
static GIT_PATH_FUNC(rebase_path_allow_rerere_autoupdate, "rebase-merge/allow_rerere_autoupdate")
134134

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+
135180
static inline int is_rebase_i(const struct replay_opts *opts)
136181
{
137182
return opts->action == REPLAY_INTERACTIVE_REBASE;
@@ -700,40 +745,6 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts,
700745
return run_command(&cmd);
701746
}
702747

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-
737748
static int rest_is_empty(const struct strbuf *sb, int start)
738749
{
739750
int i, eol;
@@ -1039,7 +1050,6 @@ static int try_to_commit(struct strbuf *msg, const char *author,
10391050
struct strbuf err = STRBUF_INIT;
10401051
struct strbuf amend_msg = STRBUF_INIT;
10411052
char *amend_author = NULL;
1042-
const char *gpg_sign;
10431053
enum commit_msg_cleanup_mode cleanup;
10441054
int res = 0;
10451055

@@ -1072,16 +1082,15 @@ static int try_to_commit(struct strbuf *msg, const char *author,
10721082
}
10731083

10741084
cleanup = (flags & CLEANUP_MSG) ? COMMIT_MSG_CLEANUP_ALL :
1075-
default_msg_cleanup;
1085+
opts->default_msg_cleanup;
1086+
10761087
if (cleanup != COMMIT_MSG_CLEANUP_NONE)
10771088
strbuf_stripspace(msg, cleanup == COMMIT_MSG_CLEANUP_ALL);
10781089
if (!opts->allow_empty_message && message_is_empty(msg, cleanup)) {
10791090
res = 1; /* run 'git commit' to display error message */
10801091
goto out;
10811092
}
10821093

1083-
gpg_sign = opts->gpg_sign ? opts->gpg_sign : default_gpg_sign;
1084-
10851094
if (write_cache_as_tree(tree.hash, 0, NULL)) {
10861095
res = error(_("git write-tree failed to write a tree"));
10871096
goto out;
@@ -1095,7 +1104,7 @@ static int try_to_commit(struct strbuf *msg, const char *author,
10951104
}
10961105

10971106
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)) {
10991108
res = error(_("failed to write commit object"));
11001109
goto out;
11011110
}

‎sequencer.h‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,13 @@ enum replay_action {
1111
REPLAY_INTERACTIVE_REBASE
1212
};
1313

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+
1421
struct replay_opts {
1522
enum replay_action action;
1623

@@ -29,6 +36,7 @@ struct replay_opts {
2936
int mainline;
3037

3138
char *gpg_sign;
39+
enum commit_msg_cleanup_mode default_msg_cleanup;
3240

3341
/* Merge strategy */
3442
char *strategy;
@@ -40,6 +48,8 @@ struct replay_opts {
4048
};
4149
#define REPLAY_OPTS_INIT { -1 }
4250

51+
/* Call this to setup defaults before parsing command line options */
52+
void sequencer_init_config(struct replay_opts *opts);
4353
int sequencer_pick_revisions(struct replay_opts *opts);
4454
int sequencer_continue(struct replay_opts *opts);
4555
int sequencer_rollback(struct replay_opts *opts);
@@ -57,15 +67,6 @@ extern const char sign_off_header[];
5767

5868
void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag);
5969
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-
6970
int message_is_empty(const struct strbuf *sb,
7071
enum commit_msg_cleanup_mode cleanup_mode);
7172
int template_untouched(const struct strbuf *sb, const char *template_file,

0 commit comments

Comments
 (0)