Skip to content

Commit 3e1dd17

Browse files
peffgitster
authored andcommitted
diff: don't load color config in plumbing
The diff config callback is split into two functions: one which loads "ui" config, and one which loads "basic" config. The former chains to the latter, as the diff UI config is a superset of the plumbing config. The color.diff variable is only loaded in the UI config. However, the basic config actually chains to git_color_default_config, which loads color.ui. This doesn't actually cause any bugs, because the plumbing diff code does not actually look at the value of color.ui. However, it is somewhat nonsensical, and it makes it difficult to refactor the color code. It probably came about because there is no git_color_config to load only color config, but rather just git_color_default_config, which loads color config and chains to git_default_config. This patch splits out the color-specific portion of git_color_default_config so that the diff UI config can call it directly. This is perhaps better explained by the chaining of callbacks. Before we had: git_diff_ui_config -> git_diff_basic_config -> git_color_default_config -> git_default_config Now we have: git_diff_ui_config -> git_color_config -> git_diff_basic_config -> git_default_config Signed-off-by: Jeff King <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent c659f55 commit 3e1dd17

File tree

3 files changed

+16
-3
lines changed

3 files changed

+16
-3
lines changed

color.c

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,13 +204,21 @@ int want_color(int var)
204204
return var > 0;
205205
}
206206

207-
int git_color_default_config(const char *var, const char *value, void *cb)
207+
int git_color_config(const char *var, const char *value, void *cb)
208208
{
209209
if (!strcmp(var, "color.ui")) {
210210
git_use_color_default = git_config_colorbool(var, value);
211211
return 0;
212212
}
213213

214+
return 0;
215+
}
216+
217+
int git_color_default_config(const char *var, const char *value, void *cb)
218+
{
219+
if (git_color_config(var, value, cb) < 0)
220+
return -1;
221+
214222
return git_default_config(var, value, cb);
215223
}
216224

color.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,10 @@ extern const int column_colors_ansi_max;
7474
extern int color_stdout_is_tty;
7575

7676
/*
77-
* Use this instead of git_default_config if you need the value of color.ui.
77+
* Use the first one if you need only color config; the second is a convenience
78+
* if you are just going to change to git_default_config, too.
7879
*/
80+
int git_color_config(const char *var, const char *value, void *cb);
7981
int git_color_default_config(const char *var, const char *value, void *cb);
8082

8183
int git_config_colorbool(const char *var, const char *value);

diff.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,9 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)
164164
if (!strcmp(var, "diff.ignoresubmodules"))
165165
handle_ignore_submodules_arg(&default_diff_options, value);
166166

167+
if (git_color_config(var, value, cb) < 0)
168+
return -1;
169+
167170
return git_diff_basic_config(var, value, cb);
168171
}
169172

@@ -212,7 +215,7 @@ int git_diff_basic_config(const char *var, const char *value, void *cb)
212215
if (!prefixcmp(var, "submodule."))
213216
return parse_submodule_config_option(var, value);
214217

215-
return git_color_default_config(var, value, cb);
218+
return git_default_config(var, value, cb);
216219
}
217220

218221
static char *quote_two(const char *one, const char *two)

0 commit comments

Comments
 (0)