Web lists-archives.com

Re: [PATCH v2 4/4] diff: use skip_to_optional_val_default()




Christian Couder <christian.couder@xxxxxxxxx> writes:

> -	} else if (!strcmp(arg, "--submodule"))
> -		options->submodule_format = DIFF_SUBMODULE_LOG;
> -	else if (skip_prefix(arg, "--submodule=", &arg))
> +	} else if (skip_to_optional_val_default(arg, "--submodule", &arg, "log"))
>  		return parse_submodule_opt(options, arg);

It may be annoying if we later diagnose that DIFF_SUBMODULE_LOG may
not be used in the context (e.g. together with some other optoins
that are set in "options" variable) and have to throw an error
message at the user.  parse_submodule_opt() would not know if that
string "log" came from the user or substituted by the helper, and in
the latter case, "don't use --submodule=log here" is a message that
is not quite appropriate.

This may be minor enough not matter in practice, but I dunno.  I
didn't look at other hunks in this step very carefully but I would
expect that whenever you use "default" to substitute, you'll have
the same information lossage.