Skip to content

Commit

Permalink
diff: don't load color config in plumbing
Browse files Browse the repository at this point in the history
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 <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
peff authored and gitster committed Aug 19, 2011
1 parent c659f55 commit 3e1dd17
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 3 deletions.
10 changes: 9 additions & 1 deletion color.c
Expand Up @@ -204,13 +204,21 @@ int want_color(int var)
return var > 0;
}

int git_color_default_config(const char *var, const char *value, void *cb)
int git_color_config(const char *var, const char *value, void *cb)
{
if (!strcmp(var, "color.ui")) {
git_use_color_default = git_config_colorbool(var, value);
return 0;
}

return 0;
}

int git_color_default_config(const char *var, const char *value, void *cb)
{
if (git_color_config(var, value, cb) < 0)
return -1;

return git_default_config(var, value, cb);
}

Expand Down
4 changes: 3 additions & 1 deletion color.h
Expand Up @@ -74,8 +74,10 @@ extern const int column_colors_ansi_max;
extern int color_stdout_is_tty;

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

int git_config_colorbool(const char *var, const char *value);
Expand Down
5 changes: 4 additions & 1 deletion diff.c
Expand Up @@ -164,6 +164,9 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)
if (!strcmp(var, "diff.ignoresubmodules"))
handle_ignore_submodules_arg(&default_diff_options, value);

if (git_color_config(var, value, cb) < 0)
return -1;

return git_diff_basic_config(var, value, cb);
}

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

return git_color_default_config(var, value, cb);
return git_default_config(var, value, cb);
}

static char *quote_two(const char *one, const char *two)
Expand Down

0 comments on commit 3e1dd17

Please sign in to comment.