Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes:
>> @@ -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.
If we want to add a new word-oriented option to diff that is not about
coloring the word differences, is it safe and sane to reuse the same
definition? That is, "git diff --color-words" would be affected when
diff.wordregex is set to some value, so does any new word-oriented
operation we will add, and the single regex configured would be used as
the default value to define how a word would look like.
I think it makes sense; I do not think of a case offhand where you would
want to define what a word is for the purpose of coloring diffs in one
way, and would want to use a different definition for another
word-oriented operation.
>> @@ -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.
Isn't it merely giving a fallback value when attributes does not give one?
By the way, wouldn't it make sense to optimize the precontext of that hunk
by doing _something_ like:
if (!o->word_regex && strcmp(one->path, two->path))
o->word_regex = userdiff_word_regex(two);
"Something like" comes from special cases like /dev/null for new/deleted
files, etc.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
