{"thread":{"id":"52848","subject":"[PATCH v2 0/5] am: provide a replacement for \"cat .git/rebase-apply/patch\"","startedAt":"2020-02-20T14:15:26Z","lastAt":"2020-02-20T14:15:32Z","messageCount":6,"participants":["pbonzini@redhat.com"],"isPatch":true,"patchVersion":2,"patchTotal":5},"messages":[{"id":"392168","messageId":"20200220141519.28315-1-pbonzini@redhat.com","threadId":"52848","inReplyTo":null,"subject":"[PATCH v2 0/5] am: provide a replacement for \"cat .git/rebase-apply/patch\"","fromName":"","fromEmail":"pbonzini@redhat.com","sentAt":"2020-02-20T14:15:14Z","receivedAt":"2020-02-20T14:15:26Z","isPatch":true,"sender":{"key":"pbonzini@redhat.com","avatar":"https://avatars.githubusercontent.com/u/42082?v=4"},"body":"From: Paolo Bonzini <pbonzini@redhat.com>\n\nWhen \"git am --show-current-patch\" was added in commit 984913a210 (\"am:\nadd --show-current-patch\", 2018-02-12), \"git am\" started recommending it\nas a replacement for .git/rebase-merge/patch.  Unfortunately the suggestion\nis misguided; for example, the output \"git am --show-current-patch\" cannot\nbe passed to \"git apply\" if it is encoded as quoted-printable or base64.\n\nThis series adds a new mode to \"git am --show-current-patch\" in order to\nstraighten the suggestion.  \"--show-current-patch\" grows an optional\nargument, where the default behavior can now also be obtained with\n\"--show-current-patch=raw\" and \".git/rebase-apply/patch\" can be retrieved\nwith \"--show-current-patch=diff\".\n\nThis requires a little surgery in patches 1 and 2 in order to convert\n--show-current-patch from OPTION_CMDMODE to OPTION_CALLBACK.  After this,\nthe last two patches implement the new syntax and feature.\n\nThanks,\n\nPaolo\n\nv1->v2: - split testcases to a separate patch [Junio]\n\t- improve commit messages [Junio]\n\t- fix spacing in designated initializer [Junio]\n\t- use test_i18ngrep [Eric]\n\t- replace abort with BUG [Eric]\n\t- replace \"diff|raw\" with \"(diff|raw)\" in docs and help [Eric]\n\t- improve docs wording [Eric]\n\nPaolo Bonzini (5):\n  parse-options: add testcases for OPT_CMDMODE()\n  parse-options: convert \"command mode\" to a flag\n  am: convert \"resume\" variable to a struct\n  am: support --show-current-patch=raw as a synonym\n    for--show-current-patch\n  am: support --show-current-patch=diff to retrieve\n    .git/rebase-apply/patch\n\n Documentation/git-am.txt               | 10 +--\n builtin/am.c                           | 96 ++++++++++++++++++++------\n contrib/completion/git-completion.bash |  5 ++\n parse-options.c                        | 20 +++---\n parse-options.h                        |  8 +--\n t/helper/test-parse-options.c          |  2 +\n t/t0040-parse-options.sh               | 18 +++++\n t/t4150-am.sh                          | 20 ++++++\n 8 files changed, 140 insertions(+), 39 deletions(-)\n\n-- \n2.21.1\n\n"},{"id":"392169","messageId":"20200220141519.28315-2-pbonzini@redhat.com","threadId":"52848","inReplyTo":"20200220141519.28315-1-pbonzini@redhat.com","subject":"[PATCH v2 1/5] parse-options: add testcases for OPT_CMDMODE()","fromName":"","fromEmail":"pbonzini@redhat.com","sentAt":"2020-02-20T14:15:15Z","receivedAt":"2020-02-20T14:15:26Z","isPatch":true,"sender":{"key":"pbonzini@redhat.com","avatar":"https://avatars.githubusercontent.com/u/42082?v=4"},"body":"From: Paolo Bonzini <pbonzini@redhat.com>\n\nBefore modifying the implementation, ensure that general operation of\nOPT_CMDMODE() and detection of incompatible options are covered.\n\nSigned-off-by: Paolo Bonzini <pbonzini@redhat.com>\n---\nv1->v2: - split testcases to a separate patch [Junio]\n\t- use test_i18ngrep [Eric]\n\n t/helper/test-parse-options.c |  2 ++\n t/t0040-parse-options.sh      | 18 ++++++++++++++++++\n 2 files changed, 20 insertions(+)\n\ndiff --git a/t/helper/test-parse-options.c b/t/helper/test-parse-options.c\nindex af82db06ac..2051ce57db 100644\n--- a/t/helper/test-parse-options.c\n+++ b/t/helper/test-parse-options.c\n@@ -121,6 +121,8 @@ int cmd__parse_options(int argc, const char **argv)\n \t\tOPT_INTEGER('j', NULL, &integer, \"get a integer, too\"),\n \t\tOPT_MAGNITUDE('m', \"magnitude\", &magnitude, \"get a magnitude\"),\n \t\tOPT_SET_INT(0, \"set23\", &integer, \"set integer to 23\", 23),\n+\t\tOPT_CMDMODE(0, \"mode1\", &integer, \"set integer to 1 (cmdmode option)\", 1),\n+\t\tOPT_CMDMODE(0, \"mode2\", &integer, \"set integer to 2 (cmdmode option)\", 2),\n \t\tOPT_CALLBACK('L', \"length\", &integer, \"str\",\n \t\t\t\"get length of <str>\", length_callback),\n \t\tOPT_FILENAME('F', \"file\", &file, \"set file to <file>\"),\ndiff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh\nindex 9d7c7fdaa2..3483b72db4 100755\n--- a/t/t0040-parse-options.sh\n+++ b/t/t0040-parse-options.sh\n@@ -23,6 +23,8 @@ usage: test-tool parse-options <options>\n     -j <n>                get a integer, too\n     -m, --magnitude <n>   get a magnitude\n     --set23               set integer to 23\n+    --mode1               set integer to 1 (cmdmode option)\n+    --mode2               set integer to 2 (cmdmode option)\n     -L, --length <str>    get length of <str>\n     -F, --file <file>     set file to <file>\n \n@@ -324,6 +326,22 @@ test_expect_success 'OPT_NEGBIT() works' '\n \ttest-tool parse-options --expect=\"boolean: 6\" -bb --no-neg-or4\n '\n \n+test_expect_success 'OPT_CMDMODE() works' '\n+\ttest-tool parse-options --expect=\"integer: 1\" --mode1\n+'\n+\n+test_expect_success 'OPT_CMDMODE() detects incompatibility' '\n+\ttest_must_fail test-tool parse-options --mode1 --mode2 >output 2>output.err &&\n+\ttest_must_be_empty output &&\n+\ttest_i18ngrep \"incompatible with --mode\" output.err\n+'\n+\n+test_expect_success 'OPT_CMDMODE() detects incompatibility with something else' '\n+\ttest_must_fail test-tool parse-options --set23 --mode2 >output 2>output.err &&\n+\ttest_must_be_empty output &&\n+\ttest_i18ngrep \"incompatible with something else\" output.err\n+'\n+\n test_expect_success 'OPT_COUNTUP() with PARSE_OPT_NODASH works' '\n \ttest-tool parse-options --expect=\"boolean: 6\" + + + + + +\n '\n-- \n2.21.1\n\n\n"},{"id":"392170","messageId":"20200220141519.28315-3-pbonzini@redhat.com","threadId":"52848","inReplyTo":"20200220141519.28315-1-pbonzini@redhat.com","subject":"[PATCH v2 2/5] parse-options: convert \"command mode\" to a flag","fromName":"","fromEmail":"pbonzini@redhat.com","sentAt":"2020-02-20T14:15:16Z","receivedAt":"2020-02-20T14:15:27Z","isPatch":true,"sender":{"key":"pbonzini@redhat.com","avatar":"https://avatars.githubusercontent.com/u/42082?v=4"},"body":"From: Paolo Bonzini <pbonzini@redhat.com>\n\nOPTION_CMDMODE is essentially OPTION_SET_INT plus an extra check that\nthe variable had not set before.  In order to allow custom processing\nof the option, for example a \"command mode\" option that also has an\nargument, it would be nice to use OPTION_CALLBACK and not have to rewrite\nthe extra check on incompatible options.  In other words, making the\nprocessing of the option orthogonal to the \"only one of these\" behavior\nprovided by OPTION_CMDMODE.\n\nAdd a new flag that takes care of the check, and modify OPT_CMDMODE to\nuse it together with OPTION_SET_INT.  The new flag still requires that the\noption value points to an int, but any OPTION_* value can be specified as\nlong as it does not require a non-int type for opt->value.\n\nSigned-off-by: Paolo Bonzini <pbonzini@redhat.com>\n---\nv1->v2: - improve commit message [Junio]\n\n parse-options.c | 20 +++++++++-----------\n parse-options.h |  8 ++++----\n 2 files changed, 13 insertions(+), 15 deletions(-)\n\ndiff --git a/parse-options.c b/parse-options.c\nindex a0cef401fc..63d6bab60c 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -61,7 +61,7 @@ static enum parse_opt_result opt_command_mode_error(\n \t */\n \tfor (that = all_opts; that->type != OPTION_END; that++) {\n \t\tif (that == opt ||\n-\t\t    that->type != OPTION_CMDMODE ||\n+\t\t    !(that->flags & PARSE_OPT_CMDMODE) ||\n \t\t    that->value != opt->value ||\n \t\t    that->defval != *(int *)opt->value)\n \t\t\tcontinue;\n@@ -95,6 +95,14 @@ static enum parse_opt_result get_value(struct parse_opt_ctx_t *p,\n \tif (!(flags & OPT_SHORT) && p->opt && (opt->flags & PARSE_OPT_NOARG))\n \t\treturn error(_(\"%s takes no value\"), optname(opt, flags));\n \n+\t/*\n+\t * Giving the same mode option twice, although unnecessary,\n+\t * is not a grave error, so let it pass.\n+\t */\n+\tif ((opt->flags & PARSE_OPT_CMDMODE) &&\n+\t    *(int *)opt->value && *(int *)opt->value != opt->defval)\n+\t\treturn opt_command_mode_error(opt, all_opts, flags);\n+\n \tswitch (opt->type) {\n \tcase OPTION_LOWLEVEL_CALLBACK:\n \t\treturn opt->ll_callback(p, opt, NULL, unset);\n@@ -130,16 +138,6 @@ static enum parse_opt_result get_value(struct parse_opt_ctx_t *p,\n \t\t*(int *)opt->value = unset ? 0 : opt->defval;\n \t\treturn 0;\n \n-\tcase OPTION_CMDMODE:\n-\t\t/*\n-\t\t * Giving the same mode option twice, although is unnecessary,\n-\t\t * is not a grave error, so let it pass.\n-\t\t */\n-\t\tif (*(int *)opt->value && *(int *)opt->value != opt->defval)\n-\t\t\treturn opt_command_mode_error(opt, all_opts, flags);\n-\t\t*(int *)opt->value = opt->defval;\n-\t\treturn 0;\n-\n \tcase OPTION_STRING:\n \t\tif (unset)\n \t\t\t*(const char **)opt->value = NULL;\ndiff --git a/parse-options.h b/parse-options.h\nindex 1d60205881..fece5ba628 100644\n--- a/parse-options.h\n+++ b/parse-options.h\n@@ -18,7 +18,6 @@ enum parse_opt_type {\n \tOPTION_BITOP,\n \tOPTION_COUNTUP,\n \tOPTION_SET_INT,\n-\tOPTION_CMDMODE,\n \t/* options with arguments (usually) */\n \tOPTION_STRING,\n \tOPTION_INTEGER,\n@@ -47,7 +46,8 @@ enum parse_opt_option_flags {\n \tPARSE_OPT_LITERAL_ARGHELP = 64,\n \tPARSE_OPT_SHELL_EVAL = 256,\n \tPARSE_OPT_NOCOMPLETE = 512,\n-\tPARSE_OPT_COMP_ARG = 1024\n+\tPARSE_OPT_COMP_ARG = 1024,\n+\tPARSE_OPT_CMDMODE = 2048\n };\n \n enum parse_opt_result {\n@@ -168,8 +168,8 @@ struct option {\n #define OPT_BOOL(s, l, v, h)        OPT_BOOL_F(s, l, v, h, 0)\n #define OPT_HIDDEN_BOOL(s, l, v, h) { OPTION_SET_INT, (s), (l), (v), NULL, \\\n \t\t\t\t      (h), PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, NULL, 1}\n-#define OPT_CMDMODE(s, l, v, h, i)  { OPTION_CMDMODE, (s), (l), (v), NULL, \\\n-\t\t\t\t      (h), PARSE_OPT_NOARG|PARSE_OPT_NONEG, NULL, (i) }\n+#define OPT_CMDMODE(s, l, v, h, i)  { OPTION_SET_INT, (s), (l), (v), NULL, \\\n+\t\t\t\t      (h), PARSE_OPT_CMDMODE|PARSE_OPT_NOARG|PARSE_OPT_NONEG, NULL, (i) }\n #define OPT_INTEGER(s, l, v, h)     OPT_INTEGER_F(s, l, v, h, 0)\n #define OPT_MAGNITUDE(s, l, v, h)   { OPTION_MAGNITUDE, (s), (l), (v), \\\n \t\t\t\t      N_(\"n\"), (h), PARSE_OPT_NONEG }\n-- \n2.21.1\n\n\n"},{"id":"392171","messageId":"20200220141519.28315-4-pbonzini@redhat.com","threadId":"52848","inReplyTo":"20200220141519.28315-1-pbonzini@redhat.com","subject":"[PATCH v2 3/5] am: convert \"resume\" variable to a struct","fromName":"","fromEmail":"pbonzini@redhat.com","sentAt":"2020-02-20T14:15:17Z","receivedAt":"2020-02-20T14:15:29Z","isPatch":true,"sender":{"key":"pbonzini@redhat.com","avatar":"https://avatars.githubusercontent.com/u/42082?v=4"},"body":"From: Paolo Bonzini <pbonzini@redhat.com>\n\nThis will allow stashing the submode of --show-current-patch from a\ncallback function.  Using a struct will allow accessing both fields from\noutside cmd_am (through container_of).\n\nSigned-off-by: Paolo Bonzini <pbonzini@redhat.com>\n---\nv1->v2: - fix spacing in designated initializer [Junio]\n\n builtin/am.c | 32 ++++++++++++++++++--------------\n 1 file changed, 18 insertions(+), 14 deletions(-)\n\ndiff --git a/builtin/am.c b/builtin/am.c\nindex 8181c2aef3..bd3cda8bec 100644\n--- a/builtin/am.c\n+++ b/builtin/am.c\n@@ -2118,7 +2118,7 @@ static int parse_opt_patchformat(const struct option *opt, const char *arg, int\n \treturn 0;\n }\n \n-enum resume_mode {\n+enum resume_type {\n \tRESUME_FALSE = 0,\n \tRESUME_APPLY,\n \tRESUME_RESOLVED,\n@@ -2128,6 +2128,10 @@ enum resume_mode {\n \tRESUME_SHOW_PATCH\n };\n \n+struct resume_mode {\n+\tenum resume_type mode;\n+};\n+\n static int git_am_config(const char *k, const char *v, void *cb)\n {\n \tint status;\n@@ -2145,7 +2149,7 @@ int cmd_am(int argc, const char **argv, const char *prefix)\n \tint binary = -1;\n \tint keep_cr = -1;\n \tint patch_format = PATCH_FORMAT_UNKNOWN;\n-\tenum resume_mode resume = RESUME_FALSE;\n+\tstruct resume_mode resume = { .mode = RESUME_FALSE };\n \tint in_progress;\n \tint ret = 0;\n \n@@ -2214,22 +2218,22 @@ int cmd_am(int argc, const char **argv, const char *prefix)\n \t\t\tPARSE_OPT_NOARG),\n \t\tOPT_STRING(0, \"resolvemsg\", &state.resolvemsg, NULL,\n \t\t\tN_(\"override error message when patch failure occurs\")),\n-\t\tOPT_CMDMODE(0, \"continue\", &resume,\n+\t\tOPT_CMDMODE(0, \"continue\", &resume.mode,\n \t\t\tN_(\"continue applying patches after resolving a conflict\"),\n \t\t\tRESUME_RESOLVED),\n-\t\tOPT_CMDMODE('r', \"resolved\", &resume,\n+\t\tOPT_CMDMODE('r', \"resolved\", &resume.mode,\n \t\t\tN_(\"synonyms for --continue\"),\n \t\t\tRESUME_RESOLVED),\n-\t\tOPT_CMDMODE(0, \"skip\", &resume,\n+\t\tOPT_CMDMODE(0, \"skip\", &resume.mode,\n \t\t\tN_(\"skip the current patch\"),\n \t\t\tRESUME_SKIP),\n-\t\tOPT_CMDMODE(0, \"abort\", &resume,\n+\t\tOPT_CMDMODE(0, \"abort\", &resume.mode,\n \t\t\tN_(\"restore the original branch and abort the patching operation.\"),\n \t\t\tRESUME_ABORT),\n-\t\tOPT_CMDMODE(0, \"quit\", &resume,\n+\t\tOPT_CMDMODE(0, \"quit\", &resume.mode,\n \t\t\tN_(\"abort the patching operation but keep HEAD where it is.\"),\n \t\t\tRESUME_QUIT),\n-\t\tOPT_CMDMODE(0, \"show-current-patch\", &resume,\n+\t\tOPT_CMDMODE(0, \"show-current-patch\", &resume.mode,\n \t\t\tN_(\"show the patch being applied.\"),\n \t\t\tRESUME_SHOW_PATCH),\n \t\tOPT_BOOL(0, \"committer-date-is-author-date\",\n@@ -2281,12 +2285,12 @@ int cmd_am(int argc, const char **argv, const char *prefix)\n \t\t *    intend to feed us a patch but wanted to continue\n \t\t *    unattended.\n \t\t */\n-\t\tif (argc || (resume == RESUME_FALSE && !isatty(0)))\n+\t\tif (argc || (resume.mode == RESUME_FALSE && !isatty(0)))\n \t\t\tdie(_(\"previous rebase directory %s still exists but mbox given.\"),\n \t\t\t\tstate.dir);\n \n-\t\tif (resume == RESUME_FALSE)\n-\t\t\tresume = RESUME_APPLY;\n+\t\tif (resume.mode == RESUME_FALSE)\n+\t\t\tresume.mode = RESUME_APPLY;\n \n \t\tif (state.signoff == SIGNOFF_EXPLICIT)\n \t\t\tam_append_signoff(&state);\n@@ -2300,7 +2304,7 @@ int cmd_am(int argc, const char **argv, const char *prefix)\n \t\t * stray directories.\n \t\t */\n \t\tif (file_exists(state.dir) && !state.rebasing) {\n-\t\t\tif (resume == RESUME_ABORT || resume == RESUME_QUIT) {\n+\t\t\tif (resume.mode == RESUME_ABORT || resume.mode == RESUME_QUIT) {\n \t\t\t\tam_destroy(&state);\n \t\t\t\tam_state_release(&state);\n \t\t\t\treturn 0;\n@@ -2311,7 +2315,7 @@ int cmd_am(int argc, const char **argv, const char *prefix)\n \t\t\t\tstate.dir);\n \t\t}\n \n-\t\tif (resume)\n+\t\tif (resume.mode)\n \t\t\tdie(_(\"Resolve operation not in progress, we are not resuming.\"));\n \n \t\tfor (i = 0; i < argc; i++) {\n@@ -2329,7 +2333,7 @@ int cmd_am(int argc, const char **argv, const char *prefix)\n \t\targv_array_clear(&paths);\n \t}\n \n-\tswitch (resume) {\n+\tswitch (resume.mode) {\n \tcase RESUME_FALSE:\n \t\tam_run(&state, 0);\n \t\tbreak;\n-- \n2.21.1\n\n\n"},{"id":"392172","messageId":"20200220141519.28315-5-pbonzini@redhat.com","threadId":"52848","inReplyTo":"20200220141519.28315-1-pbonzini@redhat.com","subject":"[PATCH v2 4/5] am: support --show-current-patch=raw as a synonym for--show-current-patch","fromName":"","fromEmail":"pbonzini@redhat.com","sentAt":"2020-02-20T14:15:18Z","receivedAt":"2020-02-20T14:15:30Z","isPatch":true,"sender":{"key":"pbonzini@redhat.com","avatar":"https://avatars.githubusercontent.com/u/42082?v=4"},"body":"From: Paolo Bonzini <pbonzini@redhat.com>\n\nWhen \"git am --show-current-patch\" was added in commit 984913a210 (\"am:\nadd --show-current-patch\", 2018-02-12), \"git am\" started recommending it\nas a replacement for .git/rebase-merge/patch.  Unfortunately the suggestion\nis somewhat misguided; for example, the output \"git am --show-current-patch\"\ncannot be passed to \"git apply\" if it is encoded as quoted-printable or\nbase64.  To simplify worktree operations and to avoid that users poke into\n.git, it would be better if \"git am\" also provided a mode that copies\n.git/rebase-merge/patch to stdout.\n\nOne possibility could be to have completely separate options, introducing\nfor example --show-current-message (for .git/rebase-apply/NNNN)\nand --show-current-diff (for .git/rebase-apply/patch), while possibly\ndeprecating --show-current-patch.\n\nThat would even remove the need for the first two patches in the series.\nHowever, the long common prefix would have prevented using an abbreviated\noption such as \"--show\".  Therefore, I chose instead to add a string\nargument to --show-current-patch.  The new argument is optional, so that\n\"git am --show-current-patch\"'s behavior remains backwards-compatible.\n\nThe next choice to make is how to handle multiple --show-current-patch\noptions.  Right now, something like \"git am --abort --show-current-patch\"\nis rejected, and the previous suggestion would likewise have naturally\nrejected a command line like\n\n\tgit am --show-current-message --show-current-diff\n\nTherefore, I decided to also reject for example\n\n\tgit am --show-current-patch=diff --show-current-patch=raw\n\nIn other words the whole of --show-current-patch=xxx (including the\noptional argument) is treated as the command mode.  I found this to be\nmore consistent and intuitive, even though it differs from the usual\n\"last one wins\" semantics of the git command line.\n\nAdd the code to parse submodes based on the above design, where for now\n\"raw\" is the only valid submode.  \"raw\" prints the full e-mail message\njust like \"git am --show-current-patch\".\n\nSigned-off-by: Paolo Bonzini <pbonzini@redhat.com>\n---\nv1->v2: - improve commit messages [Junio]\n\t- replace abort with BUG [Eric]\n\n Documentation/git-am.txt               |  9 ++--\n builtin/am.c                           | 59 +++++++++++++++++++++++---\n contrib/completion/git-completion.bash |  5 +++\n t/t4150-am.sh                          | 10 +++++\n 4 files changed, 73 insertions(+), 10 deletions(-)\n\ndiff --git a/Documentation/git-am.txt b/Documentation/git-am.txt\nindex 11ca61b00b..590b711536 100644\n--- a/Documentation/git-am.txt\n+++ b/Documentation/git-am.txt\n@@ -16,7 +16,7 @@ SYNOPSIS\n \t [--exclude=<path>] [--include=<path>] [--reject] [-q | --quiet]\n \t [--[no-]scissors] [-S[<keyid>]] [--patch-format=<format>]\n \t [(<mbox> | <Maildir>)...]\n-'git am' (--continue | --skip | --abort | --quit | --show-current-patch)\n+'git am' (--continue | --skip | --abort | --quit | --show-current-patch[=raw])\n \n DESCRIPTION\n -----------\n@@ -176,9 +176,10 @@ default.   You can use `--no-utf8` to override this.\n \tAbort the patching operation but keep HEAD and the index\n \tuntouched.\n \n---show-current-patch::\n-\tShow the entire e-mail message \"git am\" has stopped at, because\n-\tof conflicts.\n+--show-current-patch[=raw]::\n+\tShow the raw contents of the e-mail message at which `git am`\n+\thas stopped due to conflicts.  The argument must be omitted or\n+\t`raw`.\n \n DISCUSSION\n ----------\ndiff --git a/builtin/am.c b/builtin/am.c\nindex bd3cda8bec..54b04da86d 100644\n--- a/builtin/am.c\n+++ b/builtin/am.c\n@@ -81,6 +81,10 @@ enum signoff_type {\n \tSIGNOFF_EXPLICIT /* --signoff was set on the command-line */\n };\n \n+enum show_patch_type {\n+\tSHOW_PATCH_RAW = 0,\n+};\n+\n struct am_state {\n \t/* state directory path */\n \tchar *dir;\n@@ -2061,7 +2065,7 @@ static void am_abort(struct am_state *state)\n \tam_destroy(state);\n }\n \n-static int show_patch(struct am_state *state)\n+static int show_patch(struct am_state *state, enum show_patch_type sub_mode)\n {\n \tstruct strbuf sb = STRBUF_INIT;\n \tconst char *patch_path;\n@@ -2078,7 +2082,14 @@ static int show_patch(struct am_state *state)\n \t\treturn ret;\n \t}\n \n-\tpatch_path = am_path(state, msgnum(state));\n+\tswitch (sub_mode) {\n+\tcase SHOW_PATCH_RAW:\n+\t\tpatch_path = am_path(state, msgnum(state));\n+\t\tbreak;\n+\tdefault:\n+\t\tBUG(\"invalid mode for --show-current-patch\");\n+\t}\n+\n \tlen = strbuf_read_file(&sb, patch_path, 0);\n \tif (len < 0)\n \t\tdie_errno(_(\"failed to read '%s'\"), patch_path);\n@@ -2130,8 +2141,42 @@ enum resume_type {\n \n struct resume_mode {\n \tenum resume_type mode;\n+\tenum show_patch_type sub_mode;\n };\n \n+static int parse_opt_show_current_patch(const struct option *opt, const char *arg, int unset)\n+{\n+\tint *opt_value = opt->value;\n+\tstruct resume_mode *resume = container_of(opt_value, struct resume_mode, mode);\n+\n+\t/*\n+\t * Please update $__git_showcurrentpatch in git-completion.bash\n+\t * when you add new options\n+\t */\n+\tconst char *valid_modes[] = {\n+\t\t[SHOW_PATCH_RAW] = \"raw\"\n+\t};\n+\tint new_value = SHOW_PATCH_RAW;\n+\n+\tif (arg) {\n+\t\tfor (new_value = 0; new_value < ARRAY_SIZE(valid_modes); new_value++) {\n+\t\t\tif (!strcmp(arg, valid_modes[new_value]))\n+\t\t\t\tbreak;\n+\t\t}\n+\t\tif (new_value >= ARRAY_SIZE(valid_modes))\n+\t\t\treturn error(_(\"Invalid value for --show-current-patch: %s\"), arg);\n+\t}\n+\n+\tif (resume->mode == RESUME_SHOW_PATCH && new_value != resume->sub_mode)\n+\t\treturn error(_(\"--show-current-patch=%s is incompatible with \"\n+\t\t\t       \"--show-current-patch=%s\"),\n+\t\t\t     arg, valid_modes[resume->sub_mode]);\n+\n+\tresume->mode = RESUME_SHOW_PATCH;\n+\tresume->sub_mode = new_value;\n+\treturn 0;\n+}\n+\n static int git_am_config(const char *k, const char *v, void *cb)\n {\n \tint status;\n@@ -2233,9 +2278,11 @@ int cmd_am(int argc, const char **argv, const char *prefix)\n \t\tOPT_CMDMODE(0, \"quit\", &resume.mode,\n \t\t\tN_(\"abort the patching operation but keep HEAD where it is.\"),\n \t\t\tRESUME_QUIT),\n-\t\tOPT_CMDMODE(0, \"show-current-patch\", &resume.mode,\n-\t\t\tN_(\"show the patch being applied.\"),\n-\t\t\tRESUME_SHOW_PATCH),\n+\t\t{ OPTION_CALLBACK, 0, \"show-current-patch\", &resume.mode,\n+\t\t  \"raw\",\n+\t\t  N_(\"show the patch being applied\"),\n+\t\t  PARSE_OPT_CMDMODE | PARSE_OPT_OPTARG | PARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP,\n+\t\t  parse_opt_show_current_patch, RESUME_SHOW_PATCH },\n \t\tOPT_BOOL(0, \"committer-date-is-author-date\",\n \t\t\t&state.committer_date_is_author_date,\n \t\t\tN_(\"lie about committer date\")),\n@@ -2354,7 +2401,7 @@ int cmd_am(int argc, const char **argv, const char *prefix)\n \t\tam_destroy(&state);\n \t\tbreak;\n \tcase RESUME_SHOW_PATCH:\n-\t\tret = show_patch(&state);\n+\t\tret = show_patch(&state, resume.sub_mode);\n \t\tbreak;\n \tdefault:\n \t\tBUG(\"invalid resume value\");\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 1aac5a56c0..247f34f1fa 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -1197,6 +1197,7 @@ __git_count_arguments ()\n \n __git_whitespacelist=\"nowarn warn error error-all fix\"\n __git_patchformat=\"mbox stgit stgit-series hg mboxrd\"\n+__git_showcurrentpatch=\"raw\"\n __git_am_inprogress_options=\"--skip --continue --resolved --abort --quit --show-current-patch\"\n \n _git_am ()\n@@ -1215,6 +1216,10 @@ _git_am ()\n \t\t__gitcomp \"$__git_patchformat\" \"\" \"${cur##--patch-format=}\"\n \t\treturn\n \t\t;;\n+\t--show-current-patch=*)\n+\t\t__gitcomp \"$__git_showcurrentpatch\" \"\" \"${cur##--show-current-patch=}\"\n+\t\treturn\n+\t\t;;\n \t--*)\n \t\t__gitcomp_builtin am \"\" \\\n \t\t\t\"$__git_am_inprogress_options\"\ndiff --git a/t/t4150-am.sh b/t/t4150-am.sh\nindex 4f1e24ecbe..afe456e75e 100755\n--- a/t/t4150-am.sh\n+++ b/t/t4150-am.sh\n@@ -666,6 +666,16 @@ test_expect_success 'am --show-current-patch' '\n \ttest_cmp .git/rebase-apply/0001 actual.patch\n '\n \n+test_expect_success 'am --show-current-patch=raw' '\n+\tgit am --show-current-patch=raw >actual.patch &&\n+\ttest_cmp .git/rebase-apply/0001 actual.patch\n+'\n+\n+test_expect_success 'am accepts repeated --show-current-patch' '\n+\tgit am --show-current-patch --show-current-patch=raw >actual.patch &&\n+\ttest_cmp .git/rebase-apply/0001 actual.patch\n+'\n+\n test_expect_success 'am --skip works' '\n \techo goodbye >expected &&\n \tgit am --skip &&\n-- \n2.21.1\n\n\n"},{"id":"392173","messageId":"20200220141519.28315-6-pbonzini@redhat.com","threadId":"52848","inReplyTo":"20200220141519.28315-1-pbonzini@redhat.com","subject":"[PATCH v2 5/5] am: support --show-current-patch=diff to retrieve .git/rebase-apply/patch","fromName":"","fromEmail":"pbonzini@redhat.com","sentAt":"2020-02-20T14:15:19Z","receivedAt":"2020-02-20T14:15:32Z","isPatch":true,"sender":{"key":"pbonzini@redhat.com","avatar":"https://avatars.githubusercontent.com/u/42082?v=4"},"body":"From: Paolo Bonzini <pbonzini@redhat.com>\n\nWhen \"git am --show-current-patch\" was added in commit 984913a210 (\"am:\nadd --show-current-patch\", 2018-02-12), \"git am\" started recommending it\nas a replacement for .git/rebase-merge/patch.  Unfortunately the suggestion\nis somewhat misguided; for example, the output of \"git am --show-current-patch\"\ncannot be passed to \"git apply\" if it is encoded as quoted-printable\nor base64.  Add a new mode to \"git am --show-current-patch\" in order to\nstraighten the suggestion.\n\nReported-by: J. Bruce Fields <bfields@redhat.com>\nSigned-off-by: Paolo Bonzini <pbonzini@redhat.com>\n---\nv1->v2: - replace \"diff|raw\" with \"(diff|raw)\" in docs and help [Eric]\n\t- improve docs wording [Eric]\n\n Documentation/git-am.txt               | 11 ++++++-----\n builtin/am.c                           |  9 +++++++--\n contrib/completion/git-completion.bash |  2 +-\n t/t4150-am.sh                          | 10 ++++++++++\n 4 files changed, 24 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/git-am.txt b/Documentation/git-am.txt\nindex 590b711536..ab5754e05d 100644\n--- a/Documentation/git-am.txt\n+++ b/Documentation/git-am.txt\n@@ -16,7 +16,7 @@ SYNOPSIS\n \t [--exclude=<path>] [--include=<path>] [--reject] [-q | --quiet]\n \t [--[no-]scissors] [-S[<keyid>]] [--patch-format=<format>]\n \t [(<mbox> | <Maildir>)...]\n-'git am' (--continue | --skip | --abort | --quit | --show-current-patch[=raw])\n+'git am' (--continue | --skip | --abort | --quit | --show-current-patch[=(diff|raw)])\n \n DESCRIPTION\n -----------\n@@ -176,10 +176,11 @@ default.   You can use `--no-utf8` to override this.\n \tAbort the patching operation but keep HEAD and the index\n \tuntouched.\n \n---show-current-patch[=raw]::\n-\tShow the raw contents of the e-mail message at which `git am`\n-\thas stopped due to conflicts.  The argument must be omitted or\n-\t`raw`.\n+--show-current-patch[=(diff|raw)]::\n+\tShow the message at which `git am` has stopped due to\n+\tconflicts.  If `raw` is specified, show the raw contents of\n+\tthe e-mail message; if `diff`, show the diff portion only.\n+\tDefaults to `raw`.\n \n DISCUSSION\n ----------\ndiff --git a/builtin/am.c b/builtin/am.c\nindex 54b04da86d..e3dfd93c25 100644\n--- a/builtin/am.c\n+++ b/builtin/am.c\n@@ -83,6 +83,7 @@ enum signoff_type {\n \n enum show_patch_type {\n \tSHOW_PATCH_RAW = 0,\n+\tSHOW_PATCH_DIFF = 1,\n };\n \n struct am_state {\n@@ -1767,7 +1768,7 @@ static void am_run(struct am_state *state, int resume)\n \t\t\t\tlinelen(state->msg), state->msg);\n \n \t\t\tif (advice_amworkdir)\n-\t\t\t\tadvise(_(\"Use 'git am --show-current-patch' to see the failed patch\"));\n+\t\t\t\tadvise(_(\"Use 'git am --show-current-patch=diff' to see the failed patch\"));\n \n \t\t\tdie_user_resolve(state);\n \t\t}\n@@ -2086,6 +2087,9 @@ static int show_patch(struct am_state *state, enum show_patch_type sub_mode)\n \tcase SHOW_PATCH_RAW:\n \t\tpatch_path = am_path(state, msgnum(state));\n \t\tbreak;\n+\tcase SHOW_PATCH_DIFF:\n+\t\tpatch_path = am_path(state, \"patch\");\n+\t\tbreak;\n \tdefault:\n \t\tBUG(\"invalid mode for --show-current-patch\");\n \t}\n@@ -2154,6 +2158,7 @@ static int parse_opt_show_current_patch(const struct option *opt, const char *ar\n \t * when you add new options\n \t */\n \tconst char *valid_modes[] = {\n+\t\t[SHOW_PATCH_DIFF] = \"diff\",\n \t\t[SHOW_PATCH_RAW] = \"raw\"\n \t};\n \tint new_value = SHOW_PATCH_RAW;\n@@ -2279,7 +2284,7 @@ int cmd_am(int argc, const char **argv, const char *prefix)\n \t\t\tN_(\"abort the patching operation but keep HEAD where it is.\"),\n \t\t\tRESUME_QUIT),\n \t\t{ OPTION_CALLBACK, 0, \"show-current-patch\", &resume.mode,\n-\t\t  \"raw\",\n+\t\t  \"(diff|raw)\",\n \t\t  N_(\"show the patch being applied\"),\n \t\t  PARSE_OPT_CMDMODE | PARSE_OPT_OPTARG | PARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP,\n \t\t  parse_opt_show_current_patch, RESUME_SHOW_PATCH },\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 247f34f1fa..1151697f01 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -1197,7 +1197,7 @@ __git_count_arguments ()\n \n __git_whitespacelist=\"nowarn warn error error-all fix\"\n __git_patchformat=\"mbox stgit stgit-series hg mboxrd\"\n-__git_showcurrentpatch=\"raw\"\n+__git_showcurrentpatch=\"diff raw\"\n __git_am_inprogress_options=\"--skip --continue --resolved --abort --quit --show-current-patch\"\n \n _git_am ()\ndiff --git a/t/t4150-am.sh b/t/t4150-am.sh\nindex afe456e75e..cb45271457 100755\n--- a/t/t4150-am.sh\n+++ b/t/t4150-am.sh\n@@ -671,11 +671,21 @@ test_expect_success 'am --show-current-patch=raw' '\n \ttest_cmp .git/rebase-apply/0001 actual.patch\n '\n \n+test_expect_success 'am --show-current-patch=diff' '\n+\tgit am --show-current-patch=diff >actual.patch &&\n+\ttest_cmp .git/rebase-apply/patch actual.patch\n+'\n+\n test_expect_success 'am accepts repeated --show-current-patch' '\n \tgit am --show-current-patch --show-current-patch=raw >actual.patch &&\n \ttest_cmp .git/rebase-apply/0001 actual.patch\n '\n \n+test_expect_success 'am detects incompatible --show-current-patch' '\n+\ttest_must_fail git am --show-current-patch=raw --show-current-patch=diff &&\n+\ttest_must_fail git am --show-current-patch --show-current-patch=diff\n+'\n+\n test_expect_success 'am --skip works' '\n \techo goodbye >expected &&\n \tgit am --skip &&\n-- \n2.21.1\n\n"}]}