On Tuesday 2009 January 20 04:02:00 you wrote: >On Mon, 19 Jan 2009, Boyd Stephen Smith Jr. wrote: >> 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. I suppose I just initialize variables by reflex, having been bitten with too many sometimes-crashes due to variables that were usually-zero. Assuming C does guarantee that it is zeroed, I'll drop the " = NULL" line noise in the next version. >> @@ -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. I don't like runtogetherwords because they are hard to read for me; I tend to choose the wrong word breaks if it is ambiguous. There are other configuration values that use camelCaseWords so I will convert over to using that. I thought "word regex" made more sense, but I wanted to match the command-line option. Will change. >> @@ -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. You are up too late Dscho. This ordering makes the config not override attributes. If one of the files has a diff driver, o->word_regex will be set to it (and become non-NULL). That will prevent execution of the body of the added "if (!o->word_regex)" -- preventing the configuration option from being used. >> 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? Will do. I expected the code changes to be larger than the test, and when I finished it was completely the other way. My next patch will be all-in-one. Thanks for your feedback. -- Boyd Stephen Smith Jr. ,= ,-_-. =. bss@xxxxxxxxxxxxxxxxx ((_/)o o(\_)) ICQ: 514984 YM/AIM: DaTwinkDaddy `-'(. .)`-' http://iguanasuicide.net/ \_/
Attachment:
signature.asc
Description: This is a digitally signed message part.
