Re: [PATCH v2 3/4] diff: use skip_to_optional_val()
From: Junio C Hamano <hidden>
Date: 2017-12-04 14:23:14
Christian Couder [off-list ref] writes:
quoted hunk
@@ -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.
quoted hunk
@@ -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.