Web lists-archives.com

Re: [PATCH] diff: differentiate error handling in parse_color_moved_ws




Junio C Hamano <gitster@xxxxxxxxx> writes:

> Stefan Beller <sbeller@xxxxxxxxxx> writes:
>
>>  
>> -static int parse_color_moved_ws(const char *arg)
>> +static unsigned parse_color_moved_ws(const char *arg)
>>  {
>>  	int ret = 0;
>>  	struct string_list l = STRING_LIST_INIT_DUP;
>> @@ -312,15 +312,19 @@ static int parse_color_moved_ws(const char *arg)
>>  			ret |= XDF_IGNORE_WHITESPACE;
>>  		else if (!strcmp(sb.buf, "allow-indentation-change"))
>>  			ret |= COLOR_MOVED_WS_ALLOW_INDENTATION_CHANGE;
>> -		else
>> +		else {
>> +			ret |= COLOR_MOVED_WS_ERROR;
>>  			error(_("ignoring unknown color-moved-ws mode '%s'"), sb.buf);
>> +		}
>> ...  
>>  	} else if (skip_prefix(arg, "--color-moved-ws=", &arg)) {
>> -		options->color_moved_ws_handling = parse_color_moved_ws(arg);
>> +		unsigned cm = parse_color_moved_ws(arg);
>> +		if (cm & COLOR_MOVED_WS_ERROR)
>> +			die("bad --color-moved-ws argument: %s", arg);
>> +		options->color_moved_ws_handling = cm;
>
> Excellent.
>
> Will queue.  Perhaps a test or two can follow to ensure a bad value
> from config does not kill while a command line does?

Wait.  This does not fix

	git -c diff.colormovedws=nonsense diff

that dies with an error message---it should ignore the config and at
moat issue a warning.

The command line handling of

	git diff --color-moved-ws=nonsense

does correctly die, but it first says "error: ignoring" before
saying "fatal: bad argument", which is suboptimal.

So, not so excellent (yet) X-<.