Christian Couder <[email protected]> writes:
> @@ -4540,13 +4535,13 @@ int diff_opt_parse(struct diff_options *options,
> return stat_opt(options, av);
>
> /* renames options */
> - else if (starts_with(arg, "-B") || starts_with(arg,
> "--break-rewrites=") ||
> - !strcmp(arg, "--break-rewrites")) {
> + else if (starts_with(arg, "-B") ||
> + skip_to_optional_val(arg, "--break-rewrites", &optarg)) {
> if ((options->break_opt = diff_scoreopt_parse(arg)) == -1)
> return error("invalid argument to -B: %s", arg+2);
> }
This is curious; "optarg" gets something, but it is not used (what
is passed to scoreopt_parse() is still "arg".
It merely is curious and not wrong; the actual parsing of the whole
thing is done in scoreopt_parse() and skip_to_optional_val() is used
merely as a substitute for "the thing must either be --break-rewrites
or must begin with --break-rewrites=" check that is done with
starts_with() and !strcmp().
It probably makes sense to allow skip_to_optional() to take a NULL
for the third parameter to clarify a callsite like this. Otherwise
the readers will wonder who consumes optarg, and why it is OK for it
to be sometimes set and sometimes left undefined.
> - else if (starts_with(arg, "-M") || starts_with(arg, "--find-renames=")
> ||
> - !strcmp(arg, "--find-renames")) {
> + else if (starts_with(arg, "-M") ||
> + skip_to_optional_val(arg, "--find-renames", &optarg)) {
Likewise.
> @@ -4554,8 +4549,8 @@ int diff_opt_parse(struct diff_options *options,
> else if (!strcmp(arg, "-D") || !strcmp(arg, "--irreversible-delete")) {
> options->irreversible_delete = 1;
> }
> - else if (starts_with(arg, "-C") || starts_with(arg, "--find-copies=") ||
> - !strcmp(arg, "--find-copies")) {
> + else if (starts_with(arg, "-C") ||
> + skip_to_optional_val(arg, "--find-copies", &optarg)) {
> if (options->detect_rename == DIFF_DETECT_COPY)
> options->flags.find_copies_harder = 1;
> if ((options->rename_score = diff_scoreopt_parse(arg)) == -1)
Likewise.
