am: simplify --show-current-patch handling

Let the parse-options code detect and handle the use of options that are incompatible with --show-current-patch. This requires exposing the distinction between the "raw" and "diff" sub-modes. Do that by splitting the mode RESUME_SHOW_PATCH into RESUME_SHOW_PATCH_RAW and RESUME_SHOW_PATCH_DIFF and stop tracking sub-modes in a separate struct. The result is a simpler callback function and more precise error messages. The original reports a spurious argument or a NULL pointer: $ git am --show-current-patch --show-current-patch=diff error: options '--show-current-patch=diff' and '--show-current-patch=raw' cannot be used together $ git am --show-current-patch=diff --show-current-patch error: options '--show-current-patch=(null)' and '--show-current-patch=diff' cannot be used together With this patch we get the more precise: $ git am --show-current-patch --show-current-patch=diff error: --show-current-patch=diff is incompatible with --show-current-patch $ git am --show-current-patch=diff --show-current-patch error: --show-current-patch is incompatible with --show-current-patch=diff Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>

René Scharfe committed Oct 28, 2023 at 13:57 UTC e5cf20e0926b1091f0e80de6e43296a2c3223dba
1 file changed +44 -68
builtin/am.c
+44 -68
@@ -92,9 +92,16 @@ enum signoff_type {
92 SIGNOFF_EXPLICIT /* --signoff was set on the command-line */
93 };
94
95 -enum show_patch_type {
96 - SHOW_PATCH_RAW = 0,
97 - SHOW_PATCH_DIFF = 1,
95 +enum resume_type {
96 + RESUME_FALSE = 0,
97 + RESUME_APPLY,
98 + RESUME_RESOLVED,
99 + RESUME_SKIP,
100 + RESUME_ABORT,
101 + RESUME_QUIT,
102 + RESUME_SHOW_PATCH_RAW,
103 + RESUME_SHOW_PATCH_DIFF,
104 + RESUME_ALLOW_EMPTY,
105 };
106
107 enum empty_action {
@@ -2191,7 +2198,7 @@ static void am_abort(struct am_state *state)
2198 am_destroy(state);
2199 }
2200
2194 -static int show_patch(struct am_state *state, enum show_patch_type sub_mode)
2201 +static int show_patch(struct am_state *state, enum resume_type resume_mode)
2202 {
2203 struct strbuf sb = STRBUF_INIT;
2204 const char *patch_path;
@@ -2206,11 +2213,11 @@ static int show_patch(struct am_state *state, enum show_patch_type sub_mode)
2213 return run_command(&cmd);
2214 }
2215
2209 - switch (sub_mode) {
2210 - case SHOW_PATCH_RAW:
2216 + switch (resume_mode) {
2217 + case RESUME_SHOW_PATCH_RAW:
2218 patch_path = am_path(state, msgnum(state));
2219 break;
2213 - case SHOW_PATCH_DIFF:
2220 + case RESUME_SHOW_PATCH_DIFF:
2221 patch_path = am_path(state, "patch");
2222 break;
2223 default:
@@ -2257,57 +2264,25 @@ static int parse_opt_patchformat(const struct option *opt, const char *arg, int
2264 return 0;
2265 }
2266
2260 -enum resume_type {
2261 - RESUME_FALSE = 0,
2262 - RESUME_APPLY,
2263 - RESUME_RESOLVED,
2264 - RESUME_SKIP,
2265 - RESUME_ABORT,
2266 - RESUME_QUIT,
2267 - RESUME_SHOW_PATCH,
2268 - RESUME_ALLOW_EMPTY,
2269 -};
2270 -
2271 -struct resume_mode {
2272 - enum resume_type mode;
2273 - enum show_patch_type sub_mode;
2274 -};
2275 -
2267 static int parse_opt_show_current_patch(const struct option *opt, const char *arg, int unset)
2268 {
2269 int *opt_value = opt->value;
2279 - struct resume_mode *resume = container_of(opt_value, struct resume_mode, mode);
2270
2271 + BUG_ON_OPT_NEG(unset);
2272 +
2273 + if (!arg)
2274 + *opt_value = opt->defval;
2275 + else if (!strcmp(arg, "raw"))
2276 + *opt_value = RESUME_SHOW_PATCH_RAW;
2277 + else if (!strcmp(arg, "diff"))
2278 + *opt_value = RESUME_SHOW_PATCH_DIFF;
2279 /*
2280 * Please update $__git_showcurrentpatch in git-completion.bash
2281 * when you add new options
2282 */
2285 - const char *valid_modes[] = {
2286 - [SHOW_PATCH_DIFF] = "diff",
2287 - [SHOW_PATCH_RAW] = "raw"
2288 - };
2289 - int new_value = SHOW_PATCH_RAW;
2290 -
2291 - BUG_ON_OPT_NEG(unset);
2292 -
2293 - if (arg) {
2294 - for (new_value = 0; new_value < ARRAY_SIZE(valid_modes); new_value++) {
2295 - if (!strcmp(arg, valid_modes[new_value]))
2296 - break;
2297 - }
2298 - if (new_value >= ARRAY_SIZE(valid_modes))
2299 - return error(_("invalid value for '%s': '%s'"),
2300 - "--show-current-patch", arg);
2301 - }
2302 -
2303 - if (resume->mode == RESUME_SHOW_PATCH && new_value != resume->sub_mode)
2304 - return error(_("options '%s=%s' and '%s=%s' "
2305 - "cannot be used together"),
2306 - "--show-current-patch", arg,
2307 - "--show-current-patch", valid_modes[resume->sub_mode]);
2308 -
2309 - resume->mode = RESUME_SHOW_PATCH;
2310 - resume->sub_mode = new_value;
2283 + else
2284 + return error(_("invalid value for '%s': '%s'"),
2285 + "--show-current-patch", arg);
2286 return 0;
2287 }
2288
@@ -2317,7 +2292,7 @@ int cmd_am(int argc, const char **argv, const char *prefix)
2292 int binary = -1;
2293 int keep_cr = -1;
2294 int patch_format = PATCH_FORMAT_UNKNOWN;
2320 - struct resume_mode resume = { .mode = RESUME_FALSE };
2295 + enum resume_type resume_mode = RESUME_FALSE;
2296 int in_progress;
2297 int ret = 0;
2298
@@ -2388,27 +2363,27 @@ int cmd_am(int argc, const char **argv, const char *prefix)
2363 PARSE_OPT_NOARG),
2364 OPT_STRING(0, "resolvemsg", &state.resolvemsg, NULL,
2365 N_("override error message when patch failure occurs")),
2391 - OPT_CMDMODE(0, "continue", &resume.mode,
2366 + OPT_CMDMODE(0, "continue", &resume_mode,
2367 N_("continue applying patches after resolving a conflict"),
2368 RESUME_RESOLVED),
2394 - OPT_CMDMODE('r', "resolved", &resume.mode,
2369 + OPT_CMDMODE('r', "resolved", &resume_mode,
2370 N_("synonyms for --continue"),
2371 RESUME_RESOLVED),
2397 - OPT_CMDMODE(0, "skip", &resume.mode,
2372 + OPT_CMDMODE(0, "skip", &resume_mode,
2373 N_("skip the current patch"),
2374 RESUME_SKIP),
2400 - OPT_CMDMODE(0, "abort", &resume.mode,
2375 + OPT_CMDMODE(0, "abort", &resume_mode,
2376 N_("restore the original branch and abort the patching operation"),
2377 RESUME_ABORT),
2403 - OPT_CMDMODE(0, "quit", &resume.mode,
2378 + OPT_CMDMODE(0, "quit", &resume_mode,
2379 N_("abort the patching operation but keep HEAD where it is"),
2380 RESUME_QUIT),
2406 - { OPTION_CALLBACK, 0, "show-current-patch", &resume.mode,
2381 + { OPTION_CALLBACK, 0, "show-current-patch", &resume_mode,
2382 "(diff|raw)",
2383 N_("show the patch being applied"),
2384 PARSE_OPT_CMDMODE | PARSE_OPT_OPTARG | PARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP,
2410 - parse_opt_show_current_patch, RESUME_SHOW_PATCH },
2411 - OPT_CMDMODE(0, "allow-empty", &resume.mode,
2385 + parse_opt_show_current_patch, RESUME_SHOW_PATCH_RAW },
2386 + OPT_CMDMODE(0, "allow-empty", &resume_mode,
2387 N_("record the empty patch as an empty commit"),
2388 RESUME_ALLOW_EMPTY),
2389 OPT_BOOL(0, "committer-date-is-author-date",
@@ -2463,12 +2438,12 @@ int cmd_am(int argc, const char **argv, const char *prefix)
2438 * intend to feed us a patch but wanted to continue
2439 * unattended.
2440 */
2466 - if (argc || (resume.mode == RESUME_FALSE && !isatty(0)))
2441 + if (argc || (resume_mode == RESUME_FALSE && !isatty(0)))
2442 die(_("previous rebase directory %s still exists but mbox given."),
2443 state.dir);
2444
2470 - if (resume.mode == RESUME_FALSE)
2471 - resume.mode = RESUME_APPLY;
2445 + if (resume_mode == RESUME_FALSE)
2446 + resume_mode = RESUME_APPLY;
2447
2448 if (state.signoff == SIGNOFF_EXPLICIT)
2449 am_append_signoff(&state);
@@ -2482,7 +2457,7 @@ int cmd_am(int argc, const char **argv, const char *prefix)
2457 * stray directories.
2458 */
2459 if (file_exists(state.dir) && !state.rebasing) {
2485 - if (resume.mode == RESUME_ABORT || resume.mode == RESUME_QUIT) {
2460 + if (resume_mode == RESUME_ABORT || resume_mode == RESUME_QUIT) {
2461 am_destroy(&state);
2462 am_state_release(&state);
2463 return 0;
@@ -2493,7 +2468,7 @@ int cmd_am(int argc, const char **argv, const char *prefix)
2468 state.dir);
2469 }
2470
2496 - if (resume.mode)
2471 + if (resume_mode)
2472 die(_("Resolve operation not in progress, we are not resuming."));
2473
2474 for (i = 0; i < argc; i++) {
@@ -2511,7 +2486,7 @@ int cmd_am(int argc, const char **argv, const char *prefix)
2486 strvec_clear(&paths);
2487 }
2488
2514 - switch (resume.mode) {
2489 + switch (resume_mode) {
2490 case RESUME_FALSE:
2491 am_run(&state, 0);
2492 break;
@@ -2520,7 +2495,7 @@ int cmd_am(int argc, const char **argv, const char *prefix)
2495 break;
2496 case RESUME_RESOLVED:
2497 case RESUME_ALLOW_EMPTY:
2523 - am_resolve(&state, resume.mode == RESUME_ALLOW_EMPTY ? 1 : 0);
2498 + am_resolve(&state, resume_mode == RESUME_ALLOW_EMPTY ? 1 : 0);
2499 break;
2500 case RESUME_SKIP:
2501 am_skip(&state);
@@ -2532,8 +2507,9 @@ int cmd_am(int argc, const char **argv, const char *prefix)
2507 am_rerere_clear();
2508 am_destroy(&state);
2509 break;
2535 - case RESUME_SHOW_PATCH:
2536 - ret = show_patch(&state, resume.sub_mode);
2510 + case RESUME_SHOW_PATCH_RAW:
2511 + case RESUME_SHOW_PATCH_DIFF:
2512 + ret = show_patch(&state, resume_mode);
2513 break;
2514 default:
2515 BUG("invalid resume value");