Re: [PATCH] diff: Support diff.color-words config option
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Jan 20, 2009, 10:02 UTC
- Message-ID
- <alpine.DEB.1.00.0901201058520.3586@pacific.mpi-cbg.de>
- In-Reply-To
- <200901192145.21115.bss@iguanasuicide.net>
Hi,
On Mon, 19 Jan 2009, Boyd Stephen Smith Jr. wrote:
Show 10 quoted lines
> diff --git a/diff.c b/diff.c > index 9fcde96..c53e1d1 100644 > --- a/diff.c > +++ b/diff.c > @@ -23,6 +23,7 @@ static int diff_detect_rename_default; > static int diff_rename_limit_default = 200; > static int diff_suppress_blank_empty; > int diff_use_color_default = -1; > +static const char *diff_color_words_cfg = NULL; > static const char *external_diff_cmd_cfg;
Guess why external_diff_cmd_cfg is not set to NULL? All variables defined outside a function are set to all-zero anyway.
Show 5 quoted lines
> @@ -92,6 +93,8 @@ int git_diff_ui_config(const char *var, const char *value, void *cb) > } > if (!strcmp(var, "diff.external")) > return git_config_string(&external_diff_cmd_cfg, var, value); > + if (!strcmp(var, "diff.color-words"))
I'd call it diff.wordregex, because that's what it is.
Show 6 quoted lines
> @@ -1550,6 +1553,8 @@ static void builtin_diff(const char *name_a, > o->word_regex = userdiff_word_regex(one); > if (!o->word_regex) > o->word_regex = userdiff_word_regex(two); > + if (!o->word_regex) > + o->word_regex = diff_color_words_cfg;
IMHO this is the wrong order. config should not override attributes, which are by definition more specific.
Show 10 quoted lines
> diff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh > index 6ebce9d..a207d9e 100755 > --- a/t/t4034-diff-words.sh > +++ b/t/t4034-diff-words.sh > @@ -105,7 +105,7 @@ a = b + c<RESET> > EOF > cp expect.non-whitespace-is-word expect > > -test_expect_failure 'use default supplied by config' ' > +test_expect_success 'use default supplied by config' '
Let's squash the two, okay?
Thanks, Dscho