Skip to content

Commit

Permalink
Revert "color: check color.ui in git_default_config()"
Browse files Browse the repository at this point in the history
This reverts commit 136c8c8.

That commit was trying to address a bug caused by 4c7f181
(make color.ui default to 'auto', 2013-06-10), in which
plumbing like diff-tree defaulted to "auto" color, but did
not respect a "color.ui" directive to disable it.

But it also meant that we started respecting "color.ui" set
to "always". This was a known problem, but 4c7f181 argued
that nobody ought to be doing that. However, that turned out
to be wrong, and we got a number of bug reports related to
"add -p" regressing in v2.14.2.

Let's revert 136c8c8, fixing the regression to "add -p".
This leaves the problem from 4c7f181 unfixed, but:

  1. It's a pretty obscure problem in the first place. I
     only noticed it while working on the color code, and we
     haven't got a single bug report or complaint about it.

  2. We can make a more moderate fix on top by respecting
     "never" but not "always" for plumbing commands. This
     is just the minimal fix to go back to the working state
     we had before v2.14.2.

Note that this isn't a pure revert. We now have a test in
t3701 which shows off the "add -p" regression. This can be
flipped to success.

Signed-off-by: Jeff King <[email protected]>
Signed-off-by: Junio C Hamano <[email protected]>
  • Loading branch information
peff authored and gitster committed Oct 17, 2017
1 parent 1d4b12f commit 33c643b
Show file tree
Hide file tree
Showing 8 changed files with 17 additions and 9 deletions.
2 changes: 1 addition & 1 deletion builtin/branch.c
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ static int git_branch_config(const char *var, const char *value, void *cb)
return config_error_nonbool(var);
return color_parse(value, branch_colors[slot]);
}
return git_default_config(var, value, cb);
return git_color_default_config(var, value, cb);
}

static const char *branch_get_color(enum color_branch ix)
Expand Down
3 changes: 2 additions & 1 deletion builtin/clean.c
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,8 @@ static int git_clean_config(const char *var, const char *value, void *cb)
return 0;
}

return git_default_config(var, value, cb);
/* inspect the color.ui config variable and others */
return git_color_default_config(var, value, cb);
}

static const char *clean_get_color(enum color_clean ix)
Expand Down
2 changes: 1 addition & 1 deletion builtin/grep.c
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ static int wait_all(void)
static int grep_cmd_config(const char *var, const char *value, void *cb)
{
int st = grep_config(var, value, cb);
if (git_default_config(var, value, cb) < 0)
if (git_color_default_config(var, value, cb) < 0)
st = -1;

if (!strcmp(var, "grep.threads")) {
Expand Down
2 changes: 1 addition & 1 deletion builtin/show-branch.c
Original file line number Diff line number Diff line change
Expand Up @@ -554,7 +554,7 @@ static int git_show_branch_config(const char *var, const char *value, void *cb)
return 0;
}

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

static int omit_in_dense(struct commit *commit, struct commit **rev, int n)
Expand Down
8 changes: 8 additions & 0 deletions color.c
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,14 @@ int git_color_config(const char *var, const char *value, void *cb)
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);
}

void color_print_strbuf(FILE *fp, const char *color, const struct strbuf *sb)
{
if (*color)
Expand Down
4 changes: 0 additions & 4 deletions config.c
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
#include "string-list.h"
#include "utf8.h"
#include "dir.h"
#include "color.h"

struct config_source {
struct config_source *prev;
Expand Down Expand Up @@ -1351,9 +1350,6 @@ int git_default_config(const char *var, const char *value, void *dummy)
if (starts_with(var, "advice."))
return git_default_advice_config(var, value);

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

if (!strcmp(var, "pager.color") || !strcmp(var, "color.pager")) {
pager_use_color = git_config_bool(var,value);
return 0;
Expand Down
3 changes: 3 additions & 0 deletions diff.c
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,9 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)
return 0;
}

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

return git_diff_basic_config(var, value, cb);
}

Expand Down
2 changes: 1 addition & 1 deletion t/t3701-add-interactive.sh
Original file line number Diff line number Diff line change
Expand Up @@ -483,7 +483,7 @@ test_expect_success 'hunk-editing handles custom comment char' '
git diff --exit-code
'

test_expect_failure 'add -p works even with color.ui=always' '
test_expect_success 'add -p works even with color.ui=always' '
git reset --hard &&
echo change >>file &&
test_config color.ui always &&
Expand Down

0 comments on commit 33c643b

Please sign in to comment.