{"thread":{"id":"55160","subject":"[PATCH 0/6][Outreachy] commit: Implementation of \"amend!\" commit","startedAt":"2021-02-17T07:31:47Z","lastAt":"2021-02-27T05:10:31Z","messageCount":44,"participants":["Charvi Mendiratta","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"417144","messageId":"20210217072904.16257-1-charvi077@gmail.com","threadId":"55160","inReplyTo":null,"subject":"[PATCH 0/6][Outreachy] commit: Implementation of \"amend!\" commit","fromName":"Charvi Mendiratta","fromEmail":"charvi077@gmail.com","sentAt":"2021-02-17T07:29:06Z","receivedAt":"2021-02-17T07:31:47Z","isPatch":true,"sender":{"key":"charvi077@gmail.com","avatar":"https://avatars.githubusercontent.com/u/32788121?v=4"},"body":"This patch series teaches `git commit --fixup` to create \"amend!\" commit\nas an alternative that works with `git rebase --autosquash`. It allows to\nfixup both the content and the commit message of the specified commit.\nHere we add two suboptions to the `--fixup`, first `amend` suboption that\ncreates an \"amend!\" commit. It takes the staged changes and also allows to\nedit the commit message of the commit we are fixing.\nExample usuage:\ngit commit --fixup=amend:\n\nSecondly, `reword` suboption that creates an empty \"amend!\" commit i.e it\nignores the staged changes and only allows to reword/edit the commit message\nof the commit we are fixing.\nExample usuage:\ngit commit --fixup=reword:\n\n** This work is rebased on the top of cm/rebase-i-updates.\n\nLink to the related discussions:\nhttps://lore.kernel.org/git/CAPSFM5f+cm87N5TO3V+rJvWyrcazybNb_Zu_bJZ+sBH4N4iyow@mail.gmail.com/\n\nCharvi Mendiratta (6):\n sequencer: export subject_length()\n commit: add amend suboption to --fixup to create amend! commit\n commit: add a reword suboption to --fixup\n t7500: add tests for --fixup[amend|reword] options\n t3437: use --fixup with options to create amend! commit\n doc/git-commit: add documentation for fixup[amend|reword] options\n\n Documentation/git-commit.txt | 39 +++++--\n Documentation/git-rebase.txt | 21 ++--\n builtin/commit.c | 97 ++++++++++++++++--\n commit.c | 14 +++\n commit.h | 3 +\n sequencer.c | 14 ---\n t/t3437-rebase-fixup-options.sh | 30 +-----\n t/t7500-commit-template-squash-signoff.sh | 118 ++++++++++++++++++++++\n 8 files changed, 271 insertions(+), 65 deletions(-)\n\n--\n2.29.0.rc1\n\n"},{"id":"417145","messageId":"20210217073725.16656-1-charvi077@gmail.com","threadId":"55160","inReplyTo":"20210217072904.16257-1-charvi077@gmail.com","subject":"[PATCH 1/6] sequencer: export subject_length()","fromName":"Charvi Mendiratta","fromEmail":"charvi077@gmail.com","sentAt":"2021-02-17T07:37:20Z","receivedAt":"2021-02-17T07:39:49Z","isPatch":true,"sender":{"key":"charvi077@gmail.com","avatar":"https://avatars.githubusercontent.com/u/32788121?v=4"},"body":"This function can be used in other parts of git. Let's move the\nfunction to commit.c.\n\nMentored-by: Christian Couder \nMentored-by: Phillip Wood \nSigned-off-by: Charvi Mendiratta \n---\n commit.c | 14 ++++++++++++++\n commit.h | 3 +++\n sequencer.c | 14 --------------\n 3 files changed, 17 insertions(+), 14 deletions(-)\n\ndiff --git a/commit.c b/commit.c\nindex bab8d5ab07..41cc72c4de 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -535,6 +535,20 @@ int find_commit_subject(const char *commit_buffer, const char **subject)\n \treturn eol - p;\n }\n \n+size_t subject_length(const char *body)\n+{\n+\tconst char *p = body;\n+\twhile (*p) {\n+\t\tconst char *next = skip_blank_lines(p);\n+\t\tif (next != p)\n+\t\t\tbreak;\n+\t\tp = strchrnul(p, '\\n');\n+\t\tif (*p)\n+\t\t\tp++;\n+\t}\n+\treturn p - body;\n+}\n+\n struct commit_list *commit_list_insert(struct commit *item, struct commit_list **list_p)\n {\n \tstruct commit_list *new_list = xmalloc(sizeof(struct commit_list));\ndiff --git a/commit.h b/commit.h\nindex f4e7b0158e..12ff6bc641 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -165,6 +165,9 @@ const void *detach_commit_buffer(struct commit *, unsigned long *sizep);\n /* Find beginning and length of commit subject. */\n int find_commit_subject(const char *commit_buffer, const char **subject);\n \n+/* Return length of the commit subject from commit log message. */\n+size_t subject_length(const char *body);\n+\n struct commit_list *commit_list_insert(struct commit *item,\n \t\t\t\t\tstruct commit_list **list);\n int commit_list_contains(struct commit *item,\ndiff --git a/sequencer.c b/sequencer.c\nindex abc6d5cdfd..3fac4ba62f 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -1724,20 +1724,6 @@ enum todo_item_flags {\n \tTODO_EDIT_FIXUP_MSG = (1 << 2),\n };\n \n-static size_t subject_length(const char *body)\n-{\n-\tconst char *p = body;\n-\twhile (*p) {\n-\t\tconst char *next = skip_blank_lines(p);\n-\t\tif (next != p)\n-\t\t\tbreak;\n-\t\tp = strchrnul(p, '\\n');\n-\t\tif (*p)\n-\t\t\tp++;\n-\t}\n-\treturn p - body;\n-}\n-\n static const char first_commit_msg_str[] = N_(\"This is the 1st commit message:\");\n static const char nth_commit_msg_fmt[] = N_(\"This is the commit message #%d:\");\n static const char skip_first_commit_msg_str[] = N_(\"The 1st commit message will be skipped:\");\n-- \n2.29.0.rc1\n\n"},{"id":"417146","messageId":"20210217073725.16656-2-charvi077@gmail.com","threadId":"55160","inReplyTo":"20210217073725.16656-1-charvi077@gmail.com","subject":"[PATCH 2/6] commit: add amend suboption to --fixup to create amend! commit","fromName":"Charvi Mendiratta","fromEmail":"charvi077@gmail.com","sentAt":"2021-02-17T07:37:21Z","receivedAt":"2021-02-17T07:40:23Z","isPatch":true,"sender":{"key":"charvi077@gmail.com","avatar":"https://avatars.githubusercontent.com/u/32788121?v=4"},"body":"`git commit --fixup=amend:` will create an \"amend!\" commit.\nThe resulting commit message subject will be \"amend! ...\" where\n\"...\" is the subject line of and the initial message\nbody will be 's message. -m can be used to override the\nmessage body.\n\nThe \"amend!\" commit when rebased with --autosquash will fixup the\ncontents and replace the commit message of with the\n\"amend!\" commit's message body.\n\nInorder to prevent rebase from creating commits with an empty\nmessage we refuse to create an \"amend!\" commit if commit message\nbody is empty.\n\nExample usage:\n$ git commit --fixup=amend:HEAD\n$ git commit --fixup=amend:HEAD -m \"clever commit message\"\n\nMentored-by: Christian Couder \nMentored-by: Phillip Wood \nSigned-off-by: Charvi Mendiratta \n---\n builtin/commit.c | 76 +++++++++++++++++++++++++++++++++++++++++++-----\n 1 file changed, 68 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 505fe60956..f2c5ad2e62 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -105,7 +105,8 @@ static const char *template_file;\n */\n static const char *author_message, *author_message_buffer;\n static char *edit_message, *use_message;\n-static char *fixup_message, *squash_message;\n+static char *fixup_message, *fixup_commit, *squash_message;\n+static const char *fixup_prefix;\n static int all, also, interactive, patch_interactive, only, amend, signoff;\n static int edit_flag = -1; /* unspecified */\n static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;\n@@ -681,6 +682,21 @@ static void adjust_comment_line_char(const struct strbuf *sb)\n \tcomment_line_char = *p;\n }\n \n+static int prepare_amend_commit(struct commit *commit, struct strbuf *sb,\n+\t\t\t\t\t\t\t\tstruct pretty_print_context *ctx) {\n+\t/*\n+\t * If we amend the 'amend!' commit then we don't want to\n+\t * duplicate the subject line.\n+\t */\n+\tconst char *format = NULL;\n+\tif (starts_with(sb->buf, \"amend! amend!\"))\n+\t\tformat = \"%b\";\n+\telse\n+\t\tformat = \"%B\";\n+\tformat_commit_message(commit, format, sb, ctx);\n+\treturn 0;\n+}\n+\n static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\t\t struct commit *current_head,\n \t\t\t struct wt_status *s,\n@@ -745,15 +761,18 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t} else if (fixup_message) {\n \t\tstruct pretty_print_context ctx = {0};\n \t\tstruct commit *commit;\n-\t\tcommit = lookup_commit_reference_by_name(fixup_message);\n+\t\tchar *fmt = xstrfmt(\"%s! %%s\\n\\n\", fixup_prefix);\n+\t\tcommit = lookup_commit_reference_by_name(fixup_commit);\n \t\tif (!commit)\n-\t\t\tdie(_(\"could not lookup commit %s\"), fixup_message);\n+\t\t\tdie(_(\"could not lookup commit %s\"), fixup_commit);\n \t\tctx.output_encoding = get_commit_output_encoding();\n-\t\tformat_commit_message(commit, \"fixup! %s\\n\\n\",\n-\t\t\t\t &sb, &ctx);\n+\t\tformat_commit_message(commit, fmt, &sb, &ctx);\n+\t\tfree(fmt);\n+\t\thook_arg1 = \"message\";\n \t\tif (have_option_m)\n \t\t\tstrbuf_addbuf(&sb, &message);\n-\t\thook_arg1 = \"message\";\n+\t\telse if (!strcmp(fixup_prefix,\"amend\"))\n+\t\t\tprepare_amend_commit(commit, &sb, &ctx);\n \t} else if (!stat(git_path_merge_msg(the_repository), &statbuf)) {\n \t\tsize_t merge_msg_start;\n \n@@ -1170,7 +1189,7 @@ static int parse_and_validate_options(int argc, const char *argv[],\n \tif (force_author && renew_authorship)\n \t\tdie(_(\"Using both --reset-author and --author does not make sense\"));\n \n-\tif (logfile || have_option_m || use_message || fixup_message)\n+\tif (logfile || have_option_m || use_message)\n \t\tuse_editor = 0;\n \tif (0 <= edit_flag)\n \t\tuse_editor = edit_flag;\n@@ -1227,6 +1246,29 @@ static int parse_and_validate_options(int argc, const char *argv[],\n \n \tif (also + only + all + interactive > 1)\n \t\tdie(_(\"Only one of --include/--only/--all/--interactive/--patch can be used.\"));\n+\n+\tif (fixup_message) {\n+\t\t/*\n+\t\t * check if ':' occurs before '^' or '@', otherwise\n+\t\t * fixup_message is a commit reference.\n+\t\t */\n+\t\tfixup_commit = strpbrk(fixup_message, \"^@:\");\n+\t\tif (fixup_commit && *fixup_commit == ':' &&\n+\t\t fixup_commit != fixup_message) {\n+\t\t\t*fixup_commit = '\\0';\n+\t\t\tif (starts_with(\"amend\", fixup_message)) {\n+\t\t\t\tfixup_prefix = \"amend\";\n+\t\t\t\tfixup_commit++;\n+\t\t\t} else {\n+\t\t\t\tdie(_(\"only amend option can be used with --fixup\"));\n+\t\t\t}\n+\t\t} else {\n+\t\t\tfixup_commit = fixup_message;\n+\t\t\tfixup_prefix = \"fixup\";\n+\t\t\tuse_editor = 0;\n+\t\t}\n+\t}\n+\n \tcleanup_mode = get_cleanup_mode(cleanup_arg, use_editor);\n \n \thandle_untracked_files_arg(s);\n@@ -1504,7 +1546,12 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\tOPT_CALLBACK('m', \"message\", &message, N_(\"message\"), N_(\"commit message\"), opt_parse_m),\n \t\tOPT_STRING('c', \"reedit-message\", &edit_message, N_(\"commit\"), N_(\"reuse and edit message from specified commit\")),\n \t\tOPT_STRING('C', \"reuse-message\", &use_message, N_(\"commit\"), N_(\"reuse message from specified commit\")),\n-\t\tOPT_STRING(0, \"fixup\", &fixup_message, N_(\"commit\"), N_(\"use autosquash formatted message to fixup specified commit\")),\n+\t\t/*\n+\t\t * TRANSLATORS: please do not translate [amend:]\n+\t\t * Here \"amend\" is an option to the --fixup command\n+\t\t * line flag, that creates amend! commit.\n+\t\t */\n+\t\tOPT_STRING(0, \"fixup\", &fixup_message, N_(\"[amend:]commit\"), N_(\"use autosquash formatted message to fixup or amend specified commit\")),\n \t\tOPT_STRING(0, \"squash\", &squash_message, N_(\"commit\"), N_(\"use autosquash formatted message to squash specified commit\")),\n \t\tOPT_BOOL(0, \"reset-author\", &renew_authorship, N_(\"the commit is authored by me now (used with -C/-c/--amend)\")),\n \t\tOPT_BOOL('s', \"signoff\", &signoff, N_(\"add a Signed-off-by trailer\")),\n@@ -1663,6 +1710,19 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\texit(1);\n \t}\n \n+\tif (fixup_message && starts_with(sb.buf, \"amend! \") &&\n+\t\t!allow_empty_message) {\n+\t\tstruct strbuf body = STRBUF_INIT;\n+\t\tsize_t len = subject_length(sb.buf);\n+\t\tstrbuf_addstr(&body, sb.buf + len);\n+\t\tif (message_is_empty(&body, cleanup_mode)) {\n+\t\t\trollback_index_files();\n+\t\t\tfprintf(stderr, _(\"Aborting commit due to empty commit message body.\\n\"));\n+\t\t\texit(1);\n+\t\t}\n+\t\tstrbuf_release(&body);\n+\t}\n+\n \tif (amend) {\n \t\tconst char *exclude_gpgsig[3] = { \"gpgsig\", \"gpgsig-sha256\", NULL };\n \t\textra = read_commit_extra_headers(current_head, exclude_gpgsig);\n-- \n2.29.0.rc1\n\n"},{"id":"417147","messageId":"20210217073725.16656-3-charvi077@gmail.com","threadId":"55160","inReplyTo":"20210217073725.16656-1-charvi077@gmail.com","subject":"[PATCH 3/6] commit: add a reword suboption to --fixup","fromName":"Charvi Mendiratta","fromEmail":"charvi077@gmail.com","sentAt":"2021-02-17T07:37:22Z","receivedAt":"2021-02-17T07:40:26Z","isPatch":true,"sender":{"key":"charvi077@gmail.com","avatar":"https://avatars.githubusercontent.com/u/32788121?v=4"},"body":"`git commit --fixup=reword:` creates an empty \"amend!\" commit\nthat will reword without changing its contents when it is\nrebased with --autosquash.\n\nApart from ignoring staged changes it works similarly to\n`--fixup=amend:`.\n\nExample usage:\n$ git commit --fixup=reword:HEAD~3\n$ git commit --fixup=reword:HEAD~3 -m \"new commit message\"\n\nMentored-by: Christian Couder \nMentored-by: Phillip Wood \nSigned-off-by: Charvi Mendiratta \n---\n builtin/commit.c | 33 +++++++++++++++++++++++++++------\n 1 file changed, 27 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex f2c5ad2e62..b5293f46d2 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1171,6 +1171,21 @@ static void finalize_deferred_config(struct wt_status *s)\n \t\ts->ahead_behind_flags = AHEAD_BEHIND_FULL;\n }\n \n+static void check_fixup_reword_options(void) {\n+\tif (whence != FROM_COMMIT) {\n+\t\tif (whence == FROM_MERGE)\n+\t\t\tdie(_(\"You are in the middle of a merge -- cannot reword.\"));\n+\t\telse if (is_from_cherry_pick(whence))\n+\t\t\tdie(_(\"You are in the middle of a cherry-pick -- cannot reword.\"));\n+\t}\n+\tif (all)\n+\t\tdie(_(\"cannot combine reword option of --fixup with --all\"));\n+\tif (also)\n+\t\tdie(_(\"cannot combine reword option of --fixup with --include\"));\n+\tif (only)\n+\t\tdie(_(\"cannot combine reword option of --fixup with --only\"));\n+}\n+\n static int parse_and_validate_options(int argc, const char *argv[],\n \t\t\t\t const struct option *options,\n \t\t\t\t const char * const usage[],\n@@ -1256,11 +1271,17 @@ static int parse_and_validate_options(int argc, const char *argv[],\n \t\tif (fixup_commit && *fixup_commit == ':' &&\n \t\t fixup_commit != fixup_message) {\n \t\t\t*fixup_commit = '\\0';\n-\t\t\tif (starts_with(\"amend\", fixup_message)) {\n+\t\t\tif (starts_with(\"amend\", fixup_message) ||\n+\t\t\t starts_with(\"reword\", fixup_message)) {\n \t\t\t\tfixup_prefix = \"amend\";\n \t\t\t\tfixup_commit++;\n+\t\t\t\tif (*fixup_message == 'r') {\n+\t\t\t\t\tcheck_fixup_reword_options();\n+\t\t\t\t\tallow_empty = 1;\n+\t\t\t\t\tonly = 1;\n+\t\t\t\t}\n \t\t\t} else {\n-\t\t\t\tdie(_(\"only amend option can be used with --fixup\"));\n+\t\t\t\tdie(_(\"only `amend` or `reword` options can be used with --fixup\"));\n \t\t\t}\n \t\t} else {\n \t\t\tfixup_commit = fixup_message;\n@@ -1547,11 +1568,11 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\tOPT_STRING('c', \"reedit-message\", &edit_message, N_(\"commit\"), N_(\"reuse and edit message from specified commit\")),\n \t\tOPT_STRING('C', \"reuse-message\", &use_message, N_(\"commit\"), N_(\"reuse message from specified commit\")),\n \t\t/*\n-\t\t * TRANSLATORS: please do not translate [amend:]\n-\t\t * Here \"amend\" is an option to the --fixup command\n-\t\t * line flag, that creates amend! commit.\n+\t\t * TRANSLATORS: please do not translate [(amend|reword):]\n+\t\t * Here \"amend\" and \"reword\" are options to the --fixup\n+\t\t * command line flag, that creates amend! commit.\n \t\t */\n-\t\tOPT_STRING(0, \"fixup\", &fixup_message, N_(\"[amend:]commit\"), N_(\"use autosquash formatted message to fixup or amend specified commit\")),\n+\t\tOPT_STRING(0, \"fixup\", &fixup_message, N_(\"[(amend|reword):]commit\"), N_(\"use autosquash formatted message to fixup or amend/reword specified commit\")),\n \t\tOPT_STRING(0, \"squash\", &squash_message, N_(\"commit\"), N_(\"use autosquash formatted message to squash specified commit\")),\n \t\tOPT_BOOL(0, \"reset-author\", &renew_authorship, N_(\"the commit is authored by me now (used with -C/-c/--amend)\")),\n \t\tOPT_BOOL('s', \"signoff\", &signoff, N_(\"add a Signed-off-by trailer\")),\n-- \n2.29.0.rc1\n\n"},{"id":"417148","messageId":"20210217073725.16656-4-charvi077@gmail.com","threadId":"55160","inReplyTo":"20210217073725.16656-1-charvi077@gmail.com","subject":"[PATCH 4/6] t7500: add tests for --fixup[amend|reword] options","fromName":"Charvi Mendiratta","fromEmail":"charvi077@gmail.com","sentAt":"2021-02-17T07:37:23Z","receivedAt":"2021-02-17T07:41:23Z","isPatch":true,"sender":{"key":"charvi077@gmail.com","avatar":"https://avatars.githubusercontent.com/u/32788121?v=4"},"body":"Mentored-by: Christian Couder \nMentored-by: Phillip Wood \nSigned-off-by: Charvi Mendiratta \n---\n t/t7500-commit-template-squash-signoff.sh | 118 ++++++++++++++++++++++\n 1 file changed, 118 insertions(+)\n\ndiff --git a/t/t7500-commit-template-squash-signoff.sh b/t/t7500-commit-template-squash-signoff.sh\nindex 6d19ece05d..d2c34019c0 100755\n--- a/t/t7500-commit-template-squash-signoff.sh\n+++ b/t/t7500-commit-template-squash-signoff.sh\n@@ -9,6 +9,8 @@ Tests for template, signoff, squash and -F functions.'\n \n . ./test-lib.sh\n \n+. \"$TEST_DIRECTORY\"/lib-rebase.sh\n+\n commit_msg_is () {\n \texpect=commit_msg_is.expect\n \tactual=commit_msg_is.actual\n@@ -279,6 +281,122 @@ test_expect_success 'commit --fixup -m\"something\" -m\"extra\"' '\n \n extra\"\n '\n+get_commit_msg () {\n+\trev=\"$1\" &&\n+\tgit log -1 --pretty=format:\"%B\" \"$rev\"\n+}\n+\n+test_expect_success 'commit --fixup=amend: creates amend! commit' '\n+\tcommit_for_rebase_autosquash_setup &&\n+\tcat >expected <<-EOF &&\n+\tamend! $(git log -1 --format=%s HEAD~)\n+\n+\t$(get_commit_msg HEAD~)\n+\n+\tedited\n+\tEOF\n+\t(\n+\t\tset_fake_editor &&\n+\t\tFAKE_COMMIT_AMEND=\"edited\" \\\n+\t\t\tgit commit --fixup=amend:HEAD~\n+\t) &&\n+\tget_commit_msg HEAD >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success '--fixup=reword: does not commit staged changes' '\n+\tcommit_for_rebase_autosquash_setup &&\n+\tcat >expected <<-EOF &&\n+\tamend! $(git log -1 --format=%s HEAD~)\n+\n+\t$(get_commit_msg HEAD~)\n+\n+\tedited\n+\tEOF\n+\t(\n+\t\tset_fake_editor &&\n+\t\tFAKE_COMMIT_AMEND=\"edited\" \\\n+\t\t\tgit commit --fixup=reword:HEAD~\n+\t) &&\n+\tget_commit_msg HEAD >actual &&\n+\ttest_cmp expected actual &&\n+\ttest_cmp_rev HEAD@{1}^{tree} HEAD^{tree} &&\n+\ttest_cmp_rev HEAD@{1} HEAD^ &&\n+\ttest_expect_code 1 git diff --cached --exit-code &&\n+\tgit cat-file blob :foo >actual &&\n+\ttest_cmp foo actual\n+'\n+\n+test_expect_success '--fixup=reword: works with -m option' '\n+\tcommit_for_rebase_autosquash_setup &&\n+\tcat >expected <<-EOF &&\n+\tamend! target message subject line\n+\n+\treword commit message\n+\tEOF\n+\tgit commit --fixup=reword:HEAD~ -m \"reword commit message\" &&\n+\tget_commit_msg HEAD >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'consecutive amend! commits remove amend! line from commit msg body' '\n+\tcommit_for_rebase_autosquash_setup &&\n+\tcat >expected <<-EOF &&\n+\tamend! amend! $(git log -1 --format=%s HEAD~)\n+\n+\t$(get_commit_msg HEAD~)\n+\n+\tedited 1\n+\n+\tedited 2\n+\tEOF\n+\techo \"reword new commit message\" >actual &&\n+\t(\n+\t\tset_fake_editor &&\n+\t\tFAKE_COMMIT_AMEND=\"edited 1\" \\\n+\t\t\tgit commit --fixup=reword:HEAD~ &&\n+\t\tFAKE_COMMIT_AMEND=\"edited 2\" \\\n+\t\t\tgit commit --fixup=reword:HEAD\n+\t) &&\n+\tget_commit_msg HEAD >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'deny to create amend! commit if its commit msg body is empty' '\n+\tcommit_for_rebase_autosquash_setup &&\n+\techo \"Aborting commit due to empty commit message body.\" >expected &&\n+\ttest_must_fail git commit --fixup=amend:HEAD~ -m \" \" 2>actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'amend! commit allows empty commit msg body with --allow-empty-message' '\n+\tcommit_for_rebase_autosquash_setup &&\n+\tcat >expected <<-EOF &&\n+\tamend! $(git log -1 --format=%s HEAD~)\n+\tEOF\n+\tgit commit --fixup=amend:HEAD~ -m \" \" --allow-empty-message &&\n+\tget_commit_msg HEAD >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_fixup_reword_opt () {\n+\ttest_expect_success C_LOCALE_OUTPUT \"--fixup=reword: incompatible with $1\" \"\n+\t\techo 'fatal: cannot combine reword option of --fixup with $1' >expect &&\n+\t\ttest_must_fail git commit --fixup=reword:HEAD~ $1 2>actual &&\n+\t\ttest_cmp expect actual\n+\t\"\n+}\n+\n+for opt in --all --include --only\n+do\n+\ttest_fixup_reword_opt $opt\n+done\n+\n+test_expect_success '--fixup=reword: -F give error message' '\n+\techo \"fatal: Only one of -c/-C/-F/--fixup can be used.\" >expect &&\n+\ttest_must_fail git commit --fixup=reword:HEAD~ -F msg 2>actual &&\n+\ttest_cmp expect actual\n+'\n \n test_expect_success 'commit --squash works with -F' '\n \tcommit_for_rebase_autosquash_setup &&\n-- \n2.29.0.rc1\n\n"},{"id":"417149","messageId":"20210217073725.16656-5-charvi077@gmail.com","threadId":"55160","inReplyTo":"20210217073725.16656-1-charvi077@gmail.com","subject":"[PATCH 5/6] t3437: use --fixup with options to create amend! commit","fromName":"Charvi Mendiratta","fromEmail":"charvi077@gmail.com","sentAt":"2021-02-17T07:37:24Z","receivedAt":"2021-02-17T07:42:28Z","isPatch":true,"sender":{"key":"charvi077@gmail.com","avatar":"https://avatars.githubusercontent.com/u/32788121?v=4"},"body":"We taught `git commit --fixup` to create \"amend!\" commit. Let's also\nupdate the tests and use it to setup the rebase tests.\n\nMentored-by: Christian Couder \nMentored-by: Phillip Wood \nSigned-off-by: Charvi Mendiratta \n---\n t/t3437-rebase-fixup-options.sh | 30 +++---------------------------\n 1 file changed, 3 insertions(+), 27 deletions(-)\n\ndiff --git a/t/t3437-rebase-fixup-options.sh b/t/t3437-rebase-fixup-options.sh\nindex a5a20354e3..d0bdc7ed02 100755\n--- a/t/t3437-rebase-fixup-options.sh\n+++ b/t/t3437-rebase-fixup-options.sh\n@@ -72,40 +72,16 @@ test_expect_success 'setup' '\n \tgit commit --fixup=HEAD -a &&\n \tgit tag B1 &&\n \ttest_tick &&\n-\tgit commit --allow-empty -F - <<-EOF &&\n-\tamend! B\n-\t$EMPTY\n-\tB\n-\t$EMPTY\n-\tedited 1\n-\tEOF\n+\tFAKE_COMMIT_AMEND=\"edited 1\" git commit --fixup=reword:B &&\n \ttest_tick &&\n-\tgit commit --allow-empty -F - <<-EOF &&\n-\tamend! amend! B\n-\t$EMPTY\n-\tB\n-\t$EMPTY\n-\tedited 1\n-\t$EMPTY\n-\tedited 2\n-\tEOF\n+\tFAKE_COMMIT_AMEND=\"edited 2\" git commit --fixup=reword:HEAD &&\n \techo B2 >B &&\n \ttest_tick &&\n \tFAKE_COMMIT_AMEND=\"edited squash\" git commit --squash=HEAD -a &&\n \tgit tag B2 &&\n \techo B3 >B &&\n \ttest_tick &&\n-\tgit commit -a -F - <<-EOF &&\n-\tamend! amend! amend! B\n-\t$EMPTY\n-\tB\n-\t$EMPTY\n-\tedited 1\n-\t$EMPTY\n-\tedited 2\n-\t$EMPTY\n-\tedited 3\n-\tEOF\n+\tFAKE_COMMIT_AMEND=\"edited 3\" git commit -a --fixup=amend:HEAD^ &&\n \tgit tag B3 &&\n \n \tGIT_AUTHOR_NAME=\"Rebase Author\" &&\n-- \n2.29.0.rc1\n\n"},{"id":"417150","messageId":"20210217073725.16656-6-charvi077@gmail.com","threadId":"55160","inReplyTo":"20210217073725.16656-1-charvi077@gmail.com","subject":"[PATCH 6/6] doc/git-commit: add documentation for fixup[amend|reword] options","fromName":"Charvi Mendiratta","fromEmail":"charvi077@gmail.com","sentAt":"2021-02-17T07:37:25Z","receivedAt":"2021-02-17T07:42:29Z","isPatch":true,"sender":{"key":"charvi077@gmail.com","avatar":"https://avatars.githubusercontent.com/u/32788121?v=4"},"body":"Mentored-by: Christian Couder \nMentored-by: Phillip Wood \nSigned-off-by: Charvi Mendiratta \n---\n Documentation/git-commit.txt | 39 ++++++++++++++++++++++++++++++------\n Documentation/git-rebase.txt | 21 ++++++++++---------\n 2 files changed, 44 insertions(+), 16 deletions(-)\n\ndiff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt\nindex 17150fa7ea..9a60876845 100644\n--- a/Documentation/git-commit.txt\n+++ b/Documentation/git-commit.txt\n@@ -9,7 +9,7 @@ SYNOPSIS\n --------\n [verse]\n 'git commit' [-a | --interactive | --patch] [-s] [-v] [-u] [--amend]\n-\t [--dry-run] [(-c | -C | --fixup | --squash) ]\n+\t [--dry-run] [(-c | -C | --squash) | --fixup [(amend|reword):])]\n \t [-F | -m ] [--reset-author] [--allow-empty]\n \t [--allow-empty-message] [--no-verify] [-e] [--author=]\n \t [--date=] [--cleanup=] [--[no-]status]\n@@ -86,11 +86,38 @@ OPTIONS\n \tLike '-C', but with `-c` the editor is invoked, so that\n \tthe user can further edit the commit message.\n \n---fixup=::\n-\tConstruct a commit message for use with `rebase --autosquash`.\n-\tThe commit message will be the subject line from the specified\n-\tcommit with a prefix of \"fixup! \". See linkgit:git-rebase[1]\n-\tfor details.\n+--fixup=[(amend|reword):]::\n+\tWhen used without options, lets's say `git commit --fixup=`,\n+\tit creates a \"fixup!\" commit where the commit message will be\n+\tthe subject line from the specified commit with a prefix of\n+\t\"fixup! \". The resulting \"fixup!\" commit is further used with\n+\t`git rebase --autosquash` to fixup the content of the specified\n+\tcommit.\n+\n+\tWhen used with option `amend`, let's say\n+\t`git commit --fixup=amend:`, it creates a \"amend!\" commit\n+\tto fixup both the content and the commit log message of the\n+\tspecified commit. The resulting \"amend!\" commit's commit message\n+\tsubject will be the subject line from the specified commit with a\n+\tprefix of \"amend! \" and the message body will be commit log message\n+\tof the specified commit. It also invokes an editor seeded with the\n+\t\"amend!\" commit log message to allow to edit further. And it denies\n+\tto create \"amend!\" commit if it's commit message body is empty unless\n+\tused with `allow-empty-message` option. \"amend!\" commit when rebased\n+\twith `--autosquash` will fixup the contents and replace the commit\n+\tmessage of the specified commit with the \"amend!\" commit's message\n+\tbody.\n+\n+\tWhen used with alternative option `reword`, let's say\n+\t`git commit --fixup=reword:`, it works similar to `amend`\n+\toption, but here it creates an empty \"amend!\" commit, i.e it does\n+\tnot take any staged changes and only allows to fixup the commit\n+\tmessage of the specified commit. It will reword the specified\n+\tcommit when it is rebased with `--autosquash`.\n+\n+\t`--fixup`, with or without option, can be used with additional\n+\tcommit message option `-m` but not with `-F`/`-c`/`-C`. See\n+\tlinkgit:git-rebase[1] for details.\n \n --squash=::\n \tConstruct a commit message for use with `rebase --autosquash`.\ndiff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt\nindex 8bfa5a9272..ffea76e53b 100644\n--- a/Documentation/git-rebase.txt\n+++ b/Documentation/git-rebase.txt\n@@ -593,16 +593,17 @@ See also INCOMPATIBLE OPTIONS below.\n \n --autosquash::\n --no-autosquash::\n-\tWhen the commit log message begins with \"squash! ...\" (or\n-\t\"fixup! ...\"), and there is already a commit in the todo list that\n-\tmatches the same `...`, automatically modify the todo list of rebase\n-\t-i so that the commit marked for squashing comes right after the\n-\tcommit to be modified, and change the action of the moved commit\n-\tfrom `pick` to `squash` (or `fixup`). A commit matches the `...` if\n-\tthe commit subject matches, or if the `...` refers to the commit's\n-\thash. As a fall-back, partial matches of the commit subject work,\n-\ttoo. The recommended way to create fixup/squash commits is by using\n-\tthe `--fixup`/`--squash` options of linkgit:git-commit[1].\n+\tWhen the commit log message begins with \"squash! ...\" (or \"fixup! ...\"\n+\tor \"amend! ...\"), and there is already a commit in the todo list that\n+\tmatches the same `...`, automatically modify the todo list of\n+\t`rebase -i`, so that the commit marked for squashing comes right after\n+\tthe commit to be modified, and change the action of the moved commit\n+\tfrom `pick` to `squash` (or `fixup` or `fixup -C`) respectively. A commit\n+\tmatches the `...` if the commit subject matches, or if the `...` refers\n+\tto the commit's hash. As a fall-back, partial matches of the commit\n+\tsubject work, too. The recommended way to create fixup/squash/amend\n+\tcommits is by using the `--fixup=[amend|reword]`/`--squash` options of\n+\tlinkgit:git-commit[1].\n +\n If the `--autosquash` option is enabled by default using the\n configuration variable `rebase.autoSquash`, this option can be\n-- \n2.29.0.rc1\n\n"},{"id":"417198","messageId":"xmqq35xulbj0.fsf@gitster.g","threadId":"55160","inReplyTo":"20210217073725.16656-2-charvi077@gmail.com","subject":"Re: [PATCH 2/6] commit: add amend suboption to --fixup to create amend! commit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-17T19:49:55Z","receivedAt":"2021-02-17T19:51:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Charvi Mendiratta writes:\n\n> `git commit --fixup=amend:` will create an \"amend!\" commit.\n> The resulting commit message subject will be \"amend! ...\" where\n> \"...\" is the subject line of and the initial message\n> body will be 's message. -m can be used to override the\n> message body.\n>\n> The \"amend!\" commit when rebased with --autosquash will fixup the\n> contents and replace the commit message of with the\n> \"amend!\" commit's message body.\n>\n> Inorder to prevent rebase from creating commits with an empty\n> message we refuse to create an \"amend!\" commit if commit message\n> body is empty.\n>\n> Example usage:\n> $ git commit --fixup=amend:HEAD\n> $ git commit --fixup=amend:HEAD -m \"clever commit message\"\n\nSorry, but it is not so clear what these examples are trying to\nillustrate.\n\nThe first one is to add a new commit to later amend the tip commit\n(It is a bit of mystery why the user does not do a more usual \"git\ncommit --amend\" right there, though, and such a mystery may distract\nreaders. If the commit were not at the tip, e.g. HEAD~3, it may be\nless distracting).\n\nThe second one, even with s|HEAD|HEAD~3| is even less clear.\nWithout the \"-m\", the resulting commit will have the subject that\nbegins with !amend but the log message body is taken from the given\ncommit, but with \"-m\", what happens? Does a single-liner 'clever\ncommit message' _replace_ the log message of the named commit,\nresulting in an !amend commit that has no message from the original?\nOr does 'clever commit message' get _appended_ the log message?\n\nI think we can just remove the \"example\" from here and explain the\nfeature well in the end-user facing documentation.\n\n> +\tif (fixup_message) {\n> +\t\t/*\n> +\t\t * check if ':' occurs before '^' or '@', otherwise\n> +\t\t * fixup_message is a commit reference.\n> +\t\t */\n\nIsn't it that you only intend to parse:\n\n --fixup\n --fixup=amend:\n --fixup=\n\nand later extend it to allow keywords other than \"amend\"?\n\nI can understand that you are trying to avoid getting fooled by\nthings like\n\n\t--fixup='HEAD^{/commit message with a colon : in it}'\n\nbut why special case only ^ and @? This feels brittle (note that I\nsaid \"things like\", exactly because I do not know if any string that\ncan name a commit must have \"@\" or \"^\" appear before \":\" if it is to\nhave \":\" in anywhere, which is what this code assumes).\n\nInstead, you can find the first colon, check for known keywords (or\na string that consists only of alnums to accomodate for future\nenhancement), and treat any garbage that happens to have a colon\nwithout the \"keyword\" as fixup_commit. I.e. something along this\nline...\n\n\t\tconst char alphas[] = \"abcde...xyz\";\n\t\tsize_t kwd_len;\n\n\t\tkwd_len = strspn(fixup_message, alphas);\n\t\tif (kwd_len && fixup_message[kwd_len] == ':') {\n\t\t\t/* found keyword? */\n\t\t\tfixup_message[kwd_len] = '\\0';\n\t\t\tif (!strcmp(\"amend\", fixup_message)) {\n\t\t\t\t... do the amend: thing ...\n#if in-next-step-when-you-add-support-for-reword\n\t\t\t} else if (!strcmp(\"reword\", fixup_message)) {\n\t\t\t\t... do the reword: thing ...\n#endif\n\t\t\t} else {\n\t\t\t\tdie(_(\"unknown --fixup=%s:\",\n\t\t\t\t\tfixup_message));\n\t\t\t}\n\t\t} else {\n\t\t\t/* the entire fixup_message is the commit */\n\t\t}\n\n"},{"id":"417204","messageId":"xmqqwnv6jwmz.fsf@gitster.g","threadId":"55160","inReplyTo":"20210217073725.16656-3-charvi077@gmail.com","subject":"Re: [PATCH 3/6] commit: add a reword suboption to --fixup","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-17T19:56:52Z","receivedAt":"2021-02-17T19:57:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Charvi Mendiratta writes:\n\n> `git commit --fixup=reword:` creates an empty \"amend!\" commit\n> that will reword without changing its contents when it is\n> rebased with --autosquash.\n>\n> Apart from ignoring staged changes it works similarly to\n> `--fixup=amend:`.\n>\n> Example usage:\n> $ git commit --fixup=reword:HEAD~3\n> $ git commit --fixup=reword:HEAD~3 -m \"new commit message\"\n\nThe same comment applies to the above as an earlier step.\n\n> +static void check_fixup_reword_options(void) {\n> +\tif (whence != FROM_COMMIT) {\n> +\t\tif (whence == FROM_MERGE)\n> +\t\t\tdie(_(\"You are in the middle of a merge -- cannot reword.\"));\n> +\t\telse if (is_from_cherry_pick(whence))\n> +\t\t\tdie(_(\"You are in the middle of a cherry-pick -- cannot reword.\"));\n> +\t}\n> +\tif (all)\n> +\t\tdie(_(\"cannot combine reword option of --fixup with --all\"));\n> +\tif (also)\n> +\t\tdie(_(\"cannot combine reword option of --fixup with --include\"));\n> +\tif (only)\n> +\t\tdie(_(\"cannot combine reword option of --fixup with --only\"));\n> +}\n\nNot just these options, wouldn't it be an error to ask to commit\nanything but an empty commit? E.g. shouldn't this sequence\n\n\tedit builtin/commit.c\n\tgit commit --fixup=reword:HEAD~3 -- builtin/commit.c\n\ntrigger an error, as we will *not* be taking any change made to the\nworking tree file?\n\nOr is that implicitly covered by some other code?\n\nIn any case, we'd need a test for that (this is just a mental note\nfor myself---I haven't finished reading the series to the end, so\nyou may have one already).\n\nThanks.\n"},{"id":"417205","messageId":"xmqqsg5ujwi2.fsf@gitster.g","threadId":"55160","inReplyTo":"20210217073725.16656-4-charvi077@gmail.com","subject":"Re: [PATCH 4/6] t7500: add tests for --fixup[amend|reword] options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-17T19:59:49Z","receivedAt":"2021-02-17T20:00:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Charvi Mendiratta writes:\n\n> Subject: Re: [PATCH 4/6] t7500: add tests for --fixup[amend|reword] options\n\nIsn't an equal '=' sign missing somewhere?\n\n> +test_fixup_reword_opt () {\n> +\ttest_expect_success C_LOCALE_OUTPUT \"--fixup=reword: incompatible with $1\" \"\n> +\t\techo 'fatal: cannot combine reword option of --fixup with $1' >expect &&\n> +\t\ttest_must_fail git commit --fixup=reword:HEAD~ $1 2>actual &&\n> +\t\ttest_cmp expect actual\n> +\t\"\n> +}\n> +\n> +for opt in --all --include --only\n> +do\n> +\ttest_fixup_reword_opt $opt\n> +done\n\nAs I suspected earlier, a pathspec is not tested here, but it should\nbe.\n\n> +test_expect_success '--fixup=reword: -F give error message' '\n> +\techo \"fatal: Only one of -c/-C/-F/--fixup can be used.\" >expect &&\n> +\ttest_must_fail git commit --fixup=reword:HEAD~ -F msg 2>actual &&\n> +\ttest_cmp expect actual\n> +'\n\nWhy? If you can use -m msg, you should be able to use -F msgfile,\ntoo, no?\n\n> test_expect_success 'commit --squash works with -F' '\n> \tcommit_for_rebase_autosquash_setup &&\n"},{"id":"417292","messageId":"CAPSFM5c0xFwnSh_O_8FRQSidQUMZWZs_f7kV8kXuCRYr0ro6rQ@mail.gmail.com","threadId":"55160","inReplyTo":"xmqqwnv6jwmz.fsf@gitster.g","subject":"Re: [PATCH 3/6] commit: add a reword suboption to --fixup","fromName":"Charvi Mendiratta","fromEmail":"charvi077@gmail.com","sentAt":"2021-02-18T10:14:14Z","receivedAt":"2021-02-18T12:24:47Z","isPatch":true,"sender":{"key":"charvi077@gmail.com","avatar":"https://avatars.githubusercontent.com/u/32788121?v=4"},"body":"On Thu, 18 Feb 2021 at 01:26, Junio C Hamano wrote:\n[...]\n> The same comment applies to the above as an earlier step.\n>\n\nOkay, will remove it.\n\n> > +static void check_fixup_reword_options(void) {\n> > + if (whence != FROM_COMMIT) {\n> > + if (whence == FROM_MERGE)\n> > + die(_(\"You are in the middle of a merge -- cannot reword.\"));\n> > + else if (is_from_cherry_pick(whence))\n> > + die(_(\"You are in the middle of a cherry-pick -- cannot reword.\"));\n> > + }\n> > + if (all)\n> > + die(_(\"cannot combine reword option of --fixup with --all\"));\n> > + if (also)\n> > + die(_(\"cannot combine reword option of --fixup with --include\"));\n> > + if (only)\n> > + die(_(\"cannot combine reword option of --fixup with --only\"));\n> > +}\n>\n> Not just these options, wouldn't it be an error to ask to commit\n> anything but an empty commit? E.g. shouldn't this sequence\n>\n> edit builtin/commit.c\n> git commit --fixup=reword:HEAD~3 -- builtin/commit.c\n>\n> trigger an error, as we will *not* be taking any change made to the\n> working tree file?\n>\n> Or is that implicitly covered by some other code?\n>\n\nI admit this is a bug here. Thanks for pointing this out and will add the\ncheck for pathspec.\n"},{"id":"417289","messageId":"CAPSFM5ddkALLCU+k+Th=pvKHEaarr_45DSn=N5DCJu1o7_5-Eg@mail.gmail.com","threadId":"55160","inReplyTo":"xmqq35xulbj0.fsf@gitster.g","subject":"Re: [PATCH 2/6] commit: add amend suboption to --fixup to create amend! commit","fromName":"Charvi Mendiratta","fromEmail":"charvi077@gmail.com","sentAt":"2021-02-18T10:13:56Z","receivedAt":"2021-02-18T12:24:51Z","isPatch":true,"sender":{"key":"charvi077@gmail.com","avatar":"https://avatars.githubusercontent.com/u/32788121?v=4"},"body":"Hi Junio,\n\nOn Thu, 18 Feb 2021 at 01:20, Junio C Hamano wrote:\n[...]\n> The second one, even with s|HEAD|HEAD~3| is even less clear.\n> Without the \"-m\", the resulting commit will have the subject that\n> begins with !amend but the log message body is taken from the given\n> commit, but with \"-m\", what happens? Does a single-liner 'clever\n> commit message' _replace_ the log message of the named commit,\n> resulting in an !amend commit that has no message from the original?\n> Or does 'clever commit message' get _appended_ the log message?\n>\n\nYes, here it gets _appended_ the log message. I agree this seems a bit\nconfusing.\n\n> I think we can just remove the \"example\" from here and explain the\n> feature well in the end-user facing documentation.\n>\n\nOkay, I will remove it from here and add it in the documentation.\n\n> > + if (fixup_message) {\n> > + /*\n> > + * check if ':' occurs before '^' or '@', otherwise\n> > + * fixup_message is a commit reference.\n> > + */\n>\n> Isn't it that you only intend to parse:\n>\n> --fixup\n> --fixup=amend:\n> --fixup=\n>\n> and later extend it to allow keywords other than \"amend\"?\n>\n\nAgree.\n\n> I can understand that you are trying to avoid getting fooled by\n> things like\n>\n> --fixup='HEAD^{/commit message with a colon : in it}'\n>\n> but why special case only ^ and @? This feels brittle (note that I\n> said \"things like\", exactly because I do not know if any string that\n> can name a commit must have \"@\" or \"^\" appear before \":\" if it is to\n> have \":\" in anywhere, which is what this code assumes).\n>\n\nOkay, I got this...\n\n> Instead, you can find the first colon, check for known keywords (or\n> a string that consists only of alnums to accomodate for future\n> enhancement), and treat any garbage that happens to have a colon\n> without the \"keyword\" as fixup_commit. I.e. something along this\n> line...\n>\n> const char alphas[] = \"abcde...xyz\";\n> size_t kwd_len;\n>\n> kwd_len = strspn(fixup_message, alphas);\n> if (kwd_len && fixup_message[kwd_len] == ':') {\n> /* found keyword? */\n> fixup_message[kwd_len] = '\\0';\n> if (!strcmp(\"amend\", fixup_message)) {\n> ... do the amend: thing ...\n> #if in-next-step-when-you-add-support-for-reword\n> } else if (!strcmp(\"reword\", fixup_message)) {\n> ... do the reword: thing ...\n> #endif\n> } else {\n> die(_(\"unknown --fixup=%s:\",\n> fixup_message));\n> }\n> } else {\n> /* the entire fixup_message is the commit */\n> }\n>\n\n...Thanks, for pointing this out. Also, in the above method for\nalnum I think we can initialize an array of alnum[] instead of\nalphas[]. Or otherwise I was thinking to instead check:\n if (!isalnum(*c) && *c == ':')\ni.e to check that first non alnum char in fixup_message is ':' and\nreturning it's position to extract both fixup_prefix and fixup_commit.\n\nWill look into it and update in the next revision.\n"},{"id":"417291","messageId":"CAPSFM5cjJ_b510wx+RtVkyxHk-aFDpOHaFRWqQuMuDf=73nBiA@mail.gmail.com","threadId":"55160","inReplyTo":"xmqqsg5ujwi2.fsf@gitster.g","subject":"Re: [PATCH 4/6] t7500: add tests for --fixup[amend|reword] options","fromName":"Charvi Mendiratta","fromEmail":"charvi077@gmail.com","sentAt":"2021-02-18T10:15:34Z","receivedAt":"2021-02-18T12:24:51Z","isPatch":true,"sender":{"key":"charvi077@gmail.com","avatar":"https://avatars.githubusercontent.com/u/32788121?v=4"},"body":"On Thu, 18 Feb 2021 at 01:29, Junio C Hamano wrote:\n>\n[...]\n> > +for opt in --all --include --only\n> > +do\n> > + test_fixup_reword_opt $opt\n> > +done\n>\n> As I suspected earlier, a pathspec is not tested here, but it should\n> be.\n>\n\nOkay, I will add it here.\n\n> > +test_expect_success '--fixup=reword: -F give error message' '\n> > + echo \"fatal: Only one of -c/-C/-F/--fixup can be used.\" >expect &&\n> > + test_must_fail git commit --fixup=reword:HEAD~ -F msg 2>actual &&\n> > + test_cmp expect actual\n> > +'\n>\n> Why? If you can use -m msg, you should be able to use -F msgfile,\n> too, no?\n\nEarlier I was thinking to let the `--fixup=amend:` use the same options as of\ncurrent `--fixup=` . But yes I agree that there should be -F option\nalso with `amend`\nand `reword`.\n\nThanks for the corrections, will do all the changes and update in the\nnext revision.\n\nThanks and Regards,\nCharvi\n"},{"id":"417341","messageId":"xmqqpn0xfal8.fsf@gitster.g","threadId":"55160","inReplyTo":"CAPSFM5ddkALLCU+k+Th=pvKHEaarr_45DSn=N5DCJu1o7_5-Eg@mail.gmail.com","subject":"Re: [PATCH 2/6] commit: add amend suboption to --fixup to create amend! commit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-18T19:18:59Z","receivedAt":"2021-02-18T19:33:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Charvi Mendiratta writes:\n\n> Hi Junio,\n>\n> On Thu, 18 Feb 2021 at 01:20, Junio C Hamano wrote:\n> [...]\n>> The second one, even with s|HEAD|HEAD~3| is even less clear.\n>> Without the \"-m\", the resulting commit will have the subject that\n>> begins with !amend but the log message body is taken from the given\n>> commit, but with \"-m\", what happens? Does a single-liner 'clever\n>> commit message' _replace_ the log message of the named commit,\n>> resulting in an !amend commit that has no message from the original?\n>> Or does 'clever commit message' get _appended_ the log message?\n>>\n>\n> Yes, here it gets _appended_ the log message. I agree this seems a bit\n> confusing.\n\nIn what situation would a user use \"-m 'appended pieces of text'\"\noption, together with \"--fixup=amend:\"? I am wondering if\nwe want such a \"append to\" feature, or is it easier to understand\nfor end-users if \"-m\", \"-F\", \"-C\" and \"-c\" (the common trait of\nthese options is that they contribute to the log message text)\nare made incompatible with --fixup=amend:.\n\n> ...Thanks, for pointing this out. Also, in the above method for\n> alnum I think we can initialize an array of alnum[] instead of\n> alphas[]. Or otherwise I was thinking to instead check:\n> if (!isalnum(*c) && *c == ':')\n\nSure a loop is fine, or alnum[] is fine, or just alpha[] is OK, I\nwould think. Do you foresee you'd need --fixup=chomp124:?\nI somehow doubt it.\n"},{"id":"417342","messageId":"xmqqh7m9fa8e.fsf@gitster.g","threadId":"55160","inReplyTo":"CAPSFM5cjJ_b510wx+RtVkyxHk-aFDpOHaFRWqQuMuDf=73nBiA@mail.gmail.com","subject":"Re: [PATCH 4/6] t7500: add tests for --fixup[amend|reword] options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-18T19:26:41Z","receivedAt":"2021-02-18T19:34:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Charvi Mendiratta writes:\n\n>> > +test_expect_success '--fixup=reword: -F give error message' '\n>> > + echo \"fatal: Only one of -c/-C/-F/--fixup can be used.\" >expect &&\n>> > + test_must_fail git commit --fixup=reword:HEAD~ -F msg 2>actual &&\n>> > + test_cmp expect actual\n>> > +'\n>>\n>> Why? If you can use -m msg, you should be able to use -F msgfile,\n>> too, no?\n>\n> Earlier I was thinking to let the `--fixup=amend:` use the same options as of\n> current `--fixup=` . But yes I agree that there should be -F option\n> also with `amend`\n> and `reword`.\n\nHmph, I was actually imagining the opposite---a context that does\nnot want to take -c/-C/-F would not want to take -m, either.\n\nWhy is -m so special, and a lot more importantly, what would a user\nwant to achieve by using \"-m more-text\" combined with this\n\"--fixup=reword:\" or \"--fixup=amend:\" feature?\n"},{"id":"417343","messageId":"xmqqlfblfae2.fsf@gitster.g","threadId":"55160","inReplyTo":"20210217073725.16656-6-charvi077@gmail.com","subject":"Re: [PATCH 6/6] doc/git-commit: add documentation for fixup[amend|reword] options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-18T19:23:17Z","receivedAt":"2021-02-18T19:34:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Charvi Mendiratta writes:\n\n> Mentored-by: Christian Couder \n> Mentored-by: Phillip Wood \n> Signed-off-by: Charvi Mendiratta \n> ---\n> Documentation/git-commit.txt | 39 ++++++++++++++++++++++++++++++------\n> Documentation/git-rebase.txt | 21 ++++++++++---------\n> 2 files changed, 44 insertions(+), 16 deletions(-)\n>\n> diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt\n> index 17150fa7ea..9a60876845 100644\n> --- a/Documentation/git-commit.txt\n> +++ b/Documentation/git-commit.txt\n> @@ -9,7 +9,7 @@ SYNOPSIS\n> --------\n> [verse]\n> 'git commit' [-a | --interactive | --patch] [-s] [-v] [-u] [--amend]\n> -\t [--dry-run] [(-c | -C | --fixup | --squash) ]\n> +\t [--dry-run] [(-c | -C | --squash) | --fixup [(amend|reword):])]\n> \t [-F | -m ] [--reset-author] [--allow-empty]\n> \t [--allow-empty-message] [--no-verify] [-e] [--author=]\n> \t [--date=] [--cleanup=] [--[no-]status]\n> @@ -86,11 +86,38 @@ OPTIONS\n> \tLike '-C', but with `-c` the editor is invoked, so that\n> \tthe user can further edit the commit message.\n> \n> ---fixup=::\n> -\tConstruct a commit message for use with `rebase --autosquash`.\n> -\tThe commit message will be the subject line from the specified\n> -\tcommit with a prefix of \"fixup! \". See linkgit:git-rebase[1]\n> -\tfor details.\n> +--fixup=[(amend|reword):]::\n> +\tWhen used without options, lets's say `git commit --fixup=`,\n> +\tit creates a \"fixup!\" commit where the commit message will be\n> +\tthe subject line from the specified commit with a prefix of\n> +\t\"fixup! \". The resulting \"fixup!\" commit is further used with\n> +\t`git rebase --autosquash` to fixup the content of the specified\n> +\tcommit.\n> +\n> +\tWhen used with option `amend`, let's say\n> +\t`git commit --fixup=amend:`, it creates a \"amend!\" commit\n> +\tto fixup both the content and the commit log message of the\n> +\tspecified commit. The resulting \"amend!\" commit's commit message\n> +\tsubject will be the subject line from the specified commit with a\n> +\tprefix of \"amend! \" and the message body will be commit log message\n> +\tof the specified commit. It also invokes an editor seeded with the\n> +\t\"amend!\" commit log message to allow to edit further. And it denies\n> +\tto create \"amend!\" commit if it's commit message body is empty unless\n> +\tused with `allow-empty-message` option. \"amend!\" commit when rebased\n> +\twith `--autosquash` will fixup the contents and replace the commit\n> +\tmessage of the specified commit with the \"amend!\" commit's message\n> +\tbody.\n> +\n> +\tWhen used with alternative option `reword`, let's say\n> +\t`git commit --fixup=reword:`, it works similar to `amend`\n> +\toption, but here it creates an empty \"amend!\" commit, i.e it does\n> +\tnot take any staged changes and only allows to fixup the commit\n> +\tmessage of the specified commit. It will reword the specified\n> +\tcommit when it is rebased with `--autosquash`.\n> +\n> +\t`--fixup`, with or without option, can be used with additional\n> +\tcommit message option `-m` but not with `-F`/`-c`/`-C`. See\n> +\tlinkgit:git-rebase[1] for details.\n\nYou must dedent the second and the subsequent paragraphs and replace\neach of these blank lines that mark inter-paragraph breaks with a\nline with a single plus '+' sign on it. See how \"is mutually\nexclusive\" explanation is appended to the description of \"-m\" as its\nsecond paragraph.\n\nSide note. It probably needs \"exclusive -> incompatible\".\n\nThe description in git-rebase.txt you touched is another good\nexample. Mimic the way its second paragraph \"If the autosquash is\nenabled...\" is formatted.\n\nThanks.\n\n> --squash=::\n> \tConstruct a commit message for use with `rebase --autosquash`.\n> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt\n> index 8bfa5a9272..ffea76e53b 100644\n> --- a/Documentation/git-rebase.txt\n> +++ b/Documentation/git-rebase.txt\n> @@ -593,16 +593,17 @@ See also INCOMPATIBLE OPTIONS below.\n> \n> --autosquash::\n> --no-autosquash::\n> -\tWhen the commit log message begins with \"squash! ...\" (or\n> -\t\"fixup! ...\"), and there is already a commit in the todo list that\n> -\tmatches the same `...`, automatically modify the todo list of rebase\n> -\t-i so that the commit marked for squashing comes right after the\n> -\tcommit to be modified, and change the action of the moved commit\n> -\tfrom `pick` to `squash` (or `fixup`). A commit matches the `...` if\n> -\tthe commit subject matches, or if the `...` refers to the commit's\n> -\thash. As a fall-back, partial matches of the commit subject work,\n> -\ttoo. The recommended way to create fixup/squash commits is by using\n> -\tthe `--fixup`/`--squash` options of linkgit:git-commit[1].\n> +\tWhen the commit log message begins with \"squash! ...\" (or \"fixup! ...\"\n> +\tor \"amend! ...\"), and there is already a commit in the todo list that\n> +\tmatches the same `...`, automatically modify the todo list of\n> +\t`rebase -i`, so that the commit marked for squashing comes right after\n> +\tthe commit to be modified, and change the action of the moved commit\n> +\tfrom `pick` to `squash` (or `fixup` or `fixup -C`) respectively. A commit\n> +\tmatches the `...` if the commit subject matches, or if the `...` refers\n> +\tto the commit's hash. As a fall-back, partial matches of the commit\n> +\tsubject work, too. The recommended way to create fixup/squash/amend\n> +\tcommits is by using the `--fixup=[amend|reword]`/`--squash` options of\n> +\tlinkgit:git-commit[1].\n> +\n> If the `--autosquash` option is enabled by default using the\n> configuration variable `rebase.autoSquash`, this option can be\n"},{"id":"417356","messageId":"xmqqpn0xdse8.fsf@gitster.g","threadId":"55160","inReplyTo":"xmqqpn0xfal8.fsf@gitster.g","subject":"Re: [PATCH 2/6] commit: add amend suboption to --fixup to create amend! commit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-18T20:37:19Z","receivedAt":"2021-02-18T20:39:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n>> ...Thanks, for pointing this out. Also, in the above method for\n>> alnum I think we can initialize an array of alnum[] instead of\n>> alphas[]. Or otherwise I was thinking to instead check:\n>> if (!isalnum(*c) && *c == ':')\n>\n> Sure a loop is fine, or alnum[] is fine, or just alpha[] is OK, I\n> would think. Do you foresee you'd need --fixup=chomp124:?\n> I somehow doubt it.\n\nHaving said that, we may regret if we did not include some\npunctuation to allow for a multi-word keyword. IOW, \"alpha plus\ndash\" might be a reasonable minimum.\n\nBut what keyword --fixup=: can take is entirely\nunder our control, so it is not all that unreasonable if we just\nforced our developers some discipline to pick a single-word keyword\nfor any of their future enhancements. It's not like we are opening\nup extensibility to the end-users, who may complain that the way\nthey can spell their new is too limited. So if we already\nhave alpha[] and/or a helper function that does strspn(alpha) that\nwe can reuse elsewhere, I do not think it is worth to try supporting\npunctuation.\n\nThanks.\n"},{"id":"417383","messageId":"CAPSFM5eJNUdzy0CA1GNjNkqL_a7ivM8qydxvHf3208nznkG9KQ@mail.gmail.com","threadId":"55160","inReplyTo":"xmqqpn0xfal8.fsf@gitster.g","subject":"Re: [PATCH 2/6] commit: add amend suboption to --fixup to create amend! commit","fromName":"Charvi Mendiratta","fromEmail":"charvi077@gmail.com","sentAt":"2021-02-19T06:09:34Z","receivedAt":"2021-02-19T06:10:46Z","isPatch":true,"sender":{"key":"charvi077@gmail.com","avatar":"https://avatars.githubusercontent.com/u/32788121?v=4"},"body":"Hi Junio,\n\nOn Fri, 19 Feb 2021 at 00:49, Junio C Hamano wrote:\n[...]\n> In what situation would a user use \"-m 'appended pieces of text'\"\n> option, together with \"--fixup=amend:\"? I am wondering if\n> we want such a \"append to\" feature, or is it easier to understand\n> for end-users if \"-m\", \"-F\", \"-C\" and \"-c\" (the common trait of\n> these options is that they contribute to the log message text)\n> are made incompatible with --fixup=amend:.\n>\n\nFor end-users \"-m\" or \"-F\" will make it easier to prepare an \"amend!\"\ncommit. Because using the \"-m\" reduces the cost of opening an editor\nto prepare \"amend!\" commit and it can be done with command line only.\nSo, I think we can keep -m/-F options.\n\n(Explained more about \"-m\" use in next thread)\n\n> > ...Thanks, for pointing this out. Also, in the above method for\n> > alnum I think we can initialize an array of alnum[] instead of\n> > alphas[]. Or otherwise I was thinking to instead check:\n> > if (!isalnum(*c) && *c == ':')\n>\n> Sure a loop is fine, or alnum[] is fine, or just alpha[] is OK, I\n> would think. Do you foresee you'd need --fixup=chomp124:?\n> I somehow doubt it.\n\nFor naming the suboptions, I don't see any use of alnum. Earlier, I\nthought that it could be possible to add the option of _commit\nname/ID_, although I am not sure about any particular use case of it\nfor future. So I thought of changing it to an alnum[] but I also agree\nthat we can use just alpha[].\n"},{"id":"417384","messageId":"CAPSFM5dEEdXwgtfz==6e_745N93kWNT4tgxRGMBmz4BPynGXKQ@mail.gmail.com","threadId":"55160","inReplyTo":"xmqqlfblfae2.fsf@gitster.g","subject":"Re: [PATCH 6/6] doc/git-commit: add documentation for fixup[amend|reword] options","fromName":"Charvi Mendiratta","fromEmail":"charvi077@gmail.com","sentAt":"2021-02-19T06:09:42Z","receivedAt":"2021-02-19T06:10:46Z","isPatch":true,"sender":{"key":"charvi077@gmail.com","avatar":"https://avatars.githubusercontent.com/u/32788121?v=4"},"body":"On Fri, 19 Feb 2021 at 00:53, Junio C Hamano wrote:\n[...]\n> > +\n> > + When used with alternative option `reword`, let's say\n> > + `git commit --fixup=reword:`, it works similar to `amend`\n> > + option, but here it creates an empty \"amend!\" commit, i.e it does\n> > + not take any staged changes and only allows to fixup the commit\n> > + message of the specified commit. It will reword the specified\n> > + commit when it is rebased with `--autosquash`.\n> > +\n> > + `--fixup`, with or without option, can be used with additional\n> > + commit message option `-m` but not with `-F`/`-c`/`-C`. See\n> > + linkgit:git-rebase[1] for details.\n>\n> You must dedent the second and the subsequent paragraphs and replace\n> each of these blank lines that mark inter-paragraph breaks with a\n> line with a single plus '+' sign on it. See how \"is mutually\n> exclusive\" explanation is appended to the description of \"-m\" as its\n> second paragraph.\n>\n> Side note. It probably needs \"exclusive -> incompatible\".\n>\n> The description in git-rebase.txt you touched is another good\n> example. Mimic the way its second paragraph \"If the autosquash is\n> enabled...\" is formatted.\n>\n\nGot it. Thanks for pointing this out, I will correct it.\n"},{"id":"417385","messageId":"CAPSFM5c_qDs3jrQaHOsLpVgFGTa_96bOooVOfjwWW4Zrc-QzEg@mail.gmail.com","threadId":"55160","inReplyTo":"xmqqh7m9fa8e.fsf@gitster.g","subject":"Re: [PATCH 4/6] t7500: add tests for --fixup[amend|reword] options","fromName":"Charvi Mendiratta","fromEmail":"charvi077@gmail.com","sentAt":"2021-02-19T06:10:36Z","receivedAt":"2021-02-19T06:11:50Z","isPatch":true,"sender":{"key":"charvi077@gmail.com","avatar":"https://avatars.githubusercontent.com/u/32788121?v=4"},"body":"On Fri, 19 Feb 2021 at 00:56, Junio C Hamano wrote:\n>\n[...]\n> Hmph, I was actually imagining the opposite---a context that does\n> not want to take -c/-C/-F would not want to take -m, either.\n>\n> Why is -m so special, and a lot more importantly, what would a user\n> want to achieve by using \"-m more-text\" combined with this\n> \"--fixup=reword:\" or \"--fixup=amend:\" feature?\n\nIf we run without '-m' option like below:\n$ git commit --fixup=reword:\n\nThen it pops the editor with default \"amend!\" commit's message i.e:\n\namend! subject of we are fixing.\n\ncommit log message of we are fixing.\n\n(Here the end-user is free to edit the above message body of \"amend!\" commit )\n\nOn the other hand, if used with -m option like below:\n$ git commit --fixup=reword: -m \"edited message\"\n\nThen it will not pop the editor and the prepared \"amend!\" commit is :\n\namend! subject of we are fixing.\n\nedited message.\n\nSo, with the \"-m\" option users can do it with the command line only.\nAnd in both the cases upon `git rebase --autosquash` the commit log\nmessage of we are fixing, will automatically be replaced by\nthe commit message body of \"amend!\" commit.\n\nHope that explains the working and I also wonder if we can improve it\nto make it more user friendly ?\n\nThanks and Regards,\nCharvi\n"},{"id":"417386","messageId":"CAPSFM5dL073pO4mDN=4ca-McFq3za09-J5hjSOpmeUkAMQseNA@mail.gmail.com","threadId":"55160","inReplyTo":"xmqqpn0xdse8.fsf@gitster.g","subject":"Re: [PATCH 2/6] commit: add amend suboption to --fixup to create amend! commit","fromName":"Charvi Mendiratta","fromEmail":"charvi077@gmail.com","sentAt":"2021-02-19T06:10:49Z","receivedAt":"2021-02-19T06:12:26Z","isPatch":true,"sender":{"key":"charvi077@gmail.com","avatar":"https://avatars.githubusercontent.com/u/32788121?v=4"},"body":"On Fri, 19 Feb 2021 at 02:07, Junio C Hamano wrote:\n>\n> Junio C Hamano writes:\n>\n> >> ...Thanks, for pointing this out. Also, in the above method for\n> >> alnum I think we can initialize an array of alnum[] instead of\n> >> alphas[]. Or otherwise I was thinking to instead check:\n> >> if (!isalnum(*c) && *c == ':')\n> >\n> > Sure a loop is fine, or alnum[] is fine, or just alpha[] is OK, I\n> > would think. Do you foresee you'd need --fixup=chomp124:?\n> > I somehow doubt it.\n>\n> Having said that, we may regret if we did not include some\n> punctuation to allow for a multi-word keyword. IOW, \"alpha plus\n> dash\" might be a reasonable minimum.\n>\n> But what keyword --fixup=: can take is entirely\n> under our control, so it is not all that unreasonable if we just\n> forced our developers some discipline to pick a single-word keyword\n> for any of their future enhancements. It's not like we are opening\n> up extensibility to the end-users, who may complain that the way\n> they can spell their new is too limited. So if we already\n> have alpha[] and/or a helper function that does strspn(alpha) that\n> we can reuse elsewhere, I do not think it is worth to try supporting\n> punctuation.\n>\n\nOh, yes punctuation is another point. Then for now maybe we can just make the\nhelper function to check for alpha[] only.\n\nThanks and Regards,\nCharvi\n"},{"id":"417425","messageId":"xmqq4ki7bf9y.fsf@gitster.g","threadId":"55160","inReplyTo":"CAPSFM5eJNUdzy0CA1GNjNkqL_a7ivM8qydxvHf3208nznkG9KQ@mail.gmail.com","subject":"Re: [PATCH 2/6] commit: add amend suboption to --fixup to create amend! commit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-20T03:15:53Z","receivedAt":"2021-02-20T03:16:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Charvi Mendiratta writes:\n\n> For end-users \"-m\" or \"-F\" will make it easier to prepare an \"amend!\"\n> commit. Because using the \"-m\" reduces the cost of opening an editor\n> to prepare \"amend!\" commit and it can be done with command line only.\n\nHmph. That is not very convicing to me. The user is motivated\nenough to fix a wrong commit log message and replace it with an\nimproved one by using the \"--fixup:amend\" feature---why would that\nimproved message can sufficiently be written with just an \"-m\nmessage\", which by definition would be much less capable of\npreparing a well-thought-out message than with an editor?\n\nYes, with \"-m\", you can add some short string easily at the end of\nthe existing commit message without opening an editor. But I am\ntrying to see why it is a good thing to be able to do so in the\nfirst place. It is very easy to make typoes while doing so and it\nwould be hard to fix these typoes, exactly because you are doing so\nwithout being in an editor. And the whole point of --fixup=amend is\nabout improving the message (as opposed to --fixup that is to record\nthe contents that have already been improved in the index).\n\nThis is why I kept asking what the use case would look like. I am\ntrying to find out if you have a good end-user story in mind that we\ncan use in the documentation to sell the feature to end-users, but I\nam not seeing one.\n\nIs the combination of \"--fixup=amend\" and \"-m \" meant to be\nused primarily \"to leave a note to myself\" when the user runs\n--fixup=amend:, to just record the points to be elaborated\nwhen the message is written for real much later? E.g.\n\n ... hack hack hack ...\n ... good stopping point, but cannot afford time to write\n ... a good log message now\n $ git commit --fixup=amend:HEAD~2 -m 'talk about X, Y and Z' -a\n ... hack hack hack ...\n ... possibly doing something entirely different ...\n ... finally comes back to finish cleaning up the branch for real ...\n $ git rebase --autosquash -i origin\n\nAnd one of the insn created in the todo sheet would be amend!, whose\ncommit message has the message taken from the original HEAD~2 PLUS the\nreminder to the user that s/he needs to talk about X, Y and Z? And\nthe user at that point writes more comprehensive message about these\nthree things?\n\nThat is a made-up example of what \"appending some short strings\npossibly full of typoes without having to open an editor\" could be\nuseful for. I am not sure if it is truly useful, or if it is just a\nhand wavy excuse not to mark -m/-F incompatible with --fixup=amend\nwithout a real merit [*].\n\n Side note: one reason why I do not find it realistic is that it\n is unlikely that the user would use --fixup=amend to slurp in\n the original log message when recording \"good stopping point,\n but cannot afford time\" fixup. \"--squash HEAD~2 -m \" would\n be much faster to record the \"note to myself\" reminder, and when\n the user finally comes back to clean things up, the amount of\n work to edit the original's message while looking at the \"note\n to myself\" appended at the end would not be any different in\n either workflow.\n\nIn any case, that was the kind of answer(s) I was looking for when I\nasked \"what is this good for?\" question.\n\nThanks.\n"},{"id":"417449","messageId":"CAPSFM5cRq9OfxypwTAsv3OFpTPM88e4_agTmFEGbaVyLi6Qbig@mail.gmail.com","threadId":"55160","inReplyTo":"xmqq4ki7bf9y.fsf@gitster.g","subject":"Re: [PATCH 2/6] commit: add amend suboption to --fixup to create amend! commit","fromName":"Charvi Mendiratta","fromEmail":"charvi077@gmail.com","sentAt":"2021-02-21T06:35:29Z","receivedAt":"2021-02-21T06:40:40Z","isPatch":true,"sender":{"key":"charvi077@gmail.com","avatar":"https://avatars.githubusercontent.com/u/32788121?v=4"},"body":"Hi,\n\nOn Sat, 20 Feb 2021 at 08:46, Junio C Hamano wrote:\n>\n> Charvi Mendiratta writes:\n>\n> > For end-users \"-m\" or \"-F\" will make it easier to prepare an \"amend!\"\n> > commit. Because using the \"-m\" reduces the cost of opening an editor\n> > to prepare \"amend!\" commit and it can be done with command line only.\n>\n> Hmph. That is not very convicing to me. The user is motivated\n> enough to fix a wrong commit log message and replace it with an\n> improved one by using the \"--fixup:amend\" feature---why would that\n> improved message can sufficiently be written with just an \"-m\n> message\", which by definition would be much less capable of\n> preparing a well-thought-out message than with an editor?\n>\n> Yes, with \"-m\", you can add some short string easily at the end of\n> the existing commit message without opening an editor. But I am\n> trying to see why it is a good thing to be able to do so in the\n> first place. It is very easy to make typoes while doing so and it\n> would be hard to fix these typoes, exactly because you are doing so\n> without being in an editor. And the whole point of --fixup=amend is\n> about improving the message (as opposed to --fixup that is to record\n> the contents that have already been improved in the index).\n>\n> This is why I kept asking what the use case would look like. I am\n> trying to find out if you have a good end-user story in mind that we\n> can use in the documentation to sell the feature to end-users, but I\n> am not seeing one.\n>\n\nI was in my mind that, as we can easily prepare the commit using `git\ncommit -m \"commit message\"`. Similarly, we can extend this working\nwith `git commit --fixup=amend: -m \"new commit message\"`. And\nthe difference is that in the later one the goal of changing the\ncommit message will be achieved after `git rebase --autosquash` i.e\nthe later command works with sequencer. This will easily allow us to\nwrite a new message for the commit we are fixing up through the\ncommand line only similar to `git commit -m` and if we need to fixup\nthe commit msg then it can be done without `-m` and the editor gets\nopen with a seeded commit message we want to fixup. Also the same\nworking applies to `reword` option also and may allow to reword the\ncommit msg from command line only.\n\nBut I am still doubtful, as I agree with above that the main concern\nis to only fixup the wrong commit message and in that case the \"-m\"\noption can't be that much productive or maybe confusing for users.\n\n> Is the combination of \"--fixup=amend\" and \"-m \" meant to be\n> used primarily \"to leave a note to myself\" when the user runs\n> --fixup=amend:, to just record the points to be elaborated\n> when the message is written for real much later? E.g.\n>\n> ... hack hack hack ...\n> ... good stopping point, but cannot afford time to write\n> ... a good log message now\n> $ git commit --fixup=amend:HEAD~2 -m 'talk about X, Y and Z' -a\n> ... hack hack hack ...\n> ... possibly doing something entirely different ...\n> ... finally comes back to finish cleaning up the branch for real ...\n> $ git rebase --autosquash -i origin\n>\n> And one of the insn created in the todo sheet would be amend!, whose\n> commit message has the message taken from the original HEAD~2 PLUS the\n> reminder to the user that s/he needs to talk about X, Y and Z? And\n> the user at that point writes more comprehensive message about these\n> three things?\n>\n\nBut here in this case, after `git rebase --autosquash -i origin`, it\nwill not open editor again and user is not allowed to write more\ncomprehensive message about these three things, unless the user\nmanually changes from automatically converted `pick` to `fixup -C`,\nto `fixup -c` command in the rebase todo list that gets opened after\n`git rebase --autosquash`. And for this case `git commit --squash` is\nmore easy to use.\n\n> That is a made-up example of what \"appending some short strings\n> possibly full of typoes without having to open an editor\" could be\n> useful for. I am not sure if it is truly useful, or if it is just a\n> hand wavy excuse not to mark -m/-F incompatible with --fixup=amend\n> without a real merit [*].\n>\n> Side note: one reason why I do not find it realistic is that it\n> is unlikely that the user would use --fixup=amend to slurp in\n> the original log message when recording \"good stopping point,\n> but cannot afford time\" fixup. \"--squash HEAD~2 -m \" would\n> be much faster to record the \"note to myself\" reminder, and when\n> the user finally comes back to clean things up, the amount of\n> work to edit the original's message while looking at the \"note\n> to myself\" appended at the end would not be any different in\n> either workflow.\n>\n\nYes, I also thought the same and agree with it.\n\n> In any case, that was the kind of answer(s) I was looking for when I\n> asked \"what is this good for?\" question.\n>\n\nThanks a lot for explaining each perspective in detail. So, now if we\nuse `-m` as an option to write side-note then `--squash` is already\navailable and for fixing the commit message opening the editor is a\nmust expected option. So shall we remove the `-m` option compatibility\nif `amend`/ `reword` suboptions are passed to `git commit --fixup` ?\n\nThanks and Regards,\nCharvi\n"},{"id":"417450","messageId":"xmqqtuq599zj.fsf@gitster.g","threadId":"55160","inReplyTo":"CAPSFM5cRq9OfxypwTAsv3OFpTPM88e4_agTmFEGbaVyLi6Qbig@mail.gmail.com","subject":"Re: [PATCH 2/6] commit: add amend suboption to --fixup to create amend! commit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-21T07:05:20Z","receivedAt":"2021-02-21T07:06:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Charvi Mendiratta writes:\n\n> Thanks a lot for explaining each perspective in detail. So, now if we\n> use `-m` as an option to write side-note then `--squash` is already\n> available and for fixing the commit message opening the editor is a\n> must expected option. So shall we remove the `-m` option compatibility\n> if `amend`/ `reword` suboptions are passed to `git commit --fixup` ?\n\nFWIW, I do not have a strong opposition against -m/-F that is not\nrejected when --fixup=amend is in use. It is OK for Git to accept\nuseless combinations of options that do not help end-users. A\ncombination that is not useful will be left unused, which is not a\ngreat loss. So, if it is more work to make the code notice when\nthese options are given in useless combinations and stop with an\nerror message than just accepting and doing useless thing, I am OK\nif we left them as they are.\n\nI was just hoping that letting \"-m \" and \"--fixup=amend\"\nused at the same time would support a great use case, and because I\ndidn't think of any, I asked the person who allowed the combination\n(that is you) what the intended use cases are. Actively supporting\na combination because it gives users great value is more satisfying\nthan just leaving useless combination to exist only because it does\nnot actively hurt. When users say \"The command can take these two\noptions at the same time, but I cannot figure out what good the\ncombination is for. It feels utterly useless to use them together\"\nit also is much easier to answer them if we can say \"because by\nusing these two together, you can do this great thing\" instead of\nhaving to say \"we do not think it makes much sense to use these two\noptions together.\"\n\nThanks.\n"},{"id":"417452","messageId":"CAPSFM5dZ=CR21eqE7Y-4AssD9h0ddnUYpy4PSzWVaf8kzsLv_g@mail.gmail.com","threadId":"55160","inReplyTo":"xmqqtuq599zj.fsf@gitster.g","subject":"Re: [PATCH 2/6] commit: add amend suboption to --fixup to create amend! commit","fromName":"Charvi Mendiratta","fromEmail":"charvi077@gmail.com","sentAt":"2021-02-21T09:20:05Z","receivedAt":"2021-02-21T09:21:14Z","isPatch":true,"sender":{"key":"charvi077@gmail.com","avatar":"https://avatars.githubusercontent.com/u/32788121?v=4"},"body":"On Sun, 21 Feb 2021 at 12:35, Junio C Hamano wrote:\n>\n> Charvi Mendiratta writes:\n>\n> > Thanks a lot for explaining each perspective in detail. So, now if we\n> > use `-m` as an option to write side-note then `--squash` is already\n> > available and for fixing the commit message opening the editor is a\n> > must expected option. So shall we remove the `-m` option compatibility\n> > if `amend`/ `reword` suboptions are passed to `git commit --fixup` ?\n>\n> FWIW, I do not have a strong opposition against -m/-F that is not\n> rejected when --fixup=amend is in use. It is OK for Git to accept\n> useless combinations of options that do not help end-users. A\n> combination that is not useful will be left unused, which is not a\n> great loss.\n\n> So, if it is more work to make the code notice when\n> these options are given in useless combinations and stop with an\n> error message\n\nSorry I didn't get this and would like to once confirm here that, are\nyou pointing to output an error message when the `-m`/`-F` option is\npassed with `git --fixup=amend/reword` ? Because I think we can do\nthis also. Otherwise ....\n\n>than just accepting and doing useless thing, I am OK\n> if we left them as they are.\n>\n\n> I was just hoping that letting \"-m \" and \"--fixup=amend\"\n> used at the same time would support a great use case, and because I\n> didn't think of any, I asked the person who allowed the combination\n> (that is you) what the intended use cases are. Actively supporting\n> a combination because it gives users great value is more satisfying\n> than just leaving useless combination to exist only because it does\n> not actively hurt. When users say \"The command can take these two\n> options at the same time, but I cannot figure out what good the\n> combination is for. It feels utterly useless to use them together\"\n> it also is much easier to answer them if we can say \"because by\n> using these two together, you can do this great thing\" instead of\n> having to say \"we do not think it makes much sense to use these two\n> options together.\"\n>\n....If we allow both `m` and `F` to work with `git commit\n--fixup=amend/reword` with the same working as it is doing now i.e to\nuse `m` to write new commit message, upon `--autosquash`, If it is\nokay? then I also agree to update the documentation more precisely and\ninclude the uses when passed with `m` /`F`(not yet added) option.\n\n(Please correct me if I am wrong)\n\nThanks and Regards,\nCharvi\n"},{"id":"417475","messageId":"xmqqft1o80pe.fsf@gitster.g","threadId":"55160","inReplyTo":"CAPSFM5dZ=CR21eqE7Y-4AssD9h0ddnUYpy4PSzWVaf8kzsLv_g@mail.gmail.com","subject":"Re: [PATCH 2/6] commit: add amend suboption to --fixup to create amend! commit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-22T17:35:41Z","receivedAt":"2021-02-22T17:36:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Charvi Mendiratta writes:\n\n>> So, if it is more work to make the code notice when\n>> these options are given in useless combinations and stop with an\n>> error message\n>\n> Sorry I didn't get this and would like to once confirm here that, are\n> you pointing to output an error message when the `-m`/`-F` option is\n> passed with `git --fixup=amend/reword` ? Because I think we can do\n> this also. Otherwise ....\n\nIf we were to make -m/-F incompatible with these new features, then\nsure, we'd notice the combination, show an error message and abort.\n\n>>than just accepting and doing useless thing, I am OK\n>> if we left them as they are.\n>\n> ....If we allow both `m` and `F` to work with `git commit\n> --fixup=amend/reword` with the same working as it is doing now i.e to\n> use `m` to write new commit message, upon `--autosquash`, If it is\n> okay? then I also agree to update the documentation more precisely and\n> include the uses when passed with `m` /`F`(not yet added) option.\n\nWhat would that more precise documentation would say, though? \n\n\"'-m message' gets appended to the message taken from the original\ncommit\"? Saying that alone, without explaining why doing such an\nappending is useful, would puzzle users and makes them ask \"but why\nsuch a useless feature exist?\" and that was why I was trying to\nfigure out what it is useful for with you, which I think we have\nfailed to do so far.\n\nMy preference at this point is to error out the combination that we\ncannot figure out how it could be useful at this moment, so that\nusers who find how it would be useful to come to us and present a\nhopefully good case for using -m with --fixup=amend:.\nI am assuming that allowing the combination at that point is easy,\nand the user request will give us a good use case we can use in the\ndocumentation to explain for what purpose a user may want to use -m\n to append a short string at the end. The end users' use case\nwe see at that point might even suggest that it would be more useful\nto prepend (as opposed to append) the message we get from -m \nto the original log message, and such a change will not be possible\nif we just choose to append without thinking through the use case we\nintend to support and release \"we do not know what good it would do\nto append with -m , but that is what the code happens to do now\"\nversion to the users, as people will depend on the behaviour of any\nreleased versions.\n"},{"id":"417533","messageId":"CAPSFM5fnxZ+WF9Df-naibeauNq1-e9=E=oh86xb9JOeeuHp3Jg@mail.gmail.com","threadId":"55160","inReplyTo":"xmqqft1o80pe.fsf@gitster.g","subject":"Re: [PATCH 2/6] commit: add amend suboption to --fixup to create amend! commit","fromName":"Charvi Mendiratta","fromEmail":"charvi077@gmail.com","sentAt":"2021-02-23T06:05:00Z","receivedAt":"2021-02-23T06:05:56Z","isPatch":true,"sender":{"key":"charvi077@gmail.com","avatar":"https://avatars.githubusercontent.com/u/32788121?v=4"},"body":"On Mon, 22 Feb 2021 at 23:05, Junio C Hamano wrote:\n[...]\n> If we were to make -m/-F incompatible with these new features, then\n> sure, we'd notice the combination, show an error message and abort.\n>\n> >>than just accepting and doing useless thing, I am OK\n> >> if we left them as they are.\n> >\n> > ....If we allow both `m` and `F` to work with `git commit\n> > --fixup=amend/reword` with the same working as it is doing now i.e to\n> > use `m` to write new commit message, upon `--autosquash`, If it is\n> > okay? then I also agree to update the documentation more precisely and\n> > include the uses when passed with `m` /`F`(not yet added) option.\n>\n> What would that more precise documentation would say, though?\n>\n> \"'-m message' gets appended to the message taken from the original\n> commit\"? Saying that alone, without explaining why doing such an\n> appending is useful, would puzzle users and makes them ask \"but why\n> such a useless feature exist?\" and that was why I was trying to\n> figure out what it is useful for with you, which I think we have\n> failed to do so far.\n>\n> My preference at this point is to error out the combination that we\n> cannot figure out how it could be useful at this moment, so that\n> users who find how it would be useful to come to us and present a\n> hopefully good case for using -m with --fixup=amend:.\n> I am assuming that allowing the combination at that point is easy,\n> and the user request will give us a good use case we can use in the\n> documentation to explain for what purpose a user may want to use -m\n> to append a short string at the end. The end users' use case\n> we see at that point might even suggest that it would be more useful\n> to prepend (as opposed to append) the message we get from -m \n> to the original log message, and such a change will not be possible\n> if we just choose to append without thinking through the use case we\n> intend to support and release \"we do not know what good it would do\n> to append with -m , but that is what the code happens to do now\"\n> version to the users, as people will depend on the behaviour of any\n> released versions.\n\nOkay, I admit prepending the msg can be another way. Thanks a lot for\nclarifying in detail, I completely agree with it and will error out\nthe combination for now.\n\nThanks and Regards,\nCharvi\n"},{"id":"417593","messageId":"xmqqeeh61rv9.fsf@gitster.g","threadId":"55160","inReplyTo":"20210217072904.16257-1-charvi077@gmail.com","subject":"Re: [PATCH 0/6][Outreachy] commit: Implementation of \"amend!\" commit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-23T19:55:22Z","receivedAt":"2021-02-23T19:56:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Charvi Mendiratta writes:\n\n> This patch series teaches `git commit --fixup` to create \"amend!\" commit\n> as an alternative that works with `git rebase --autosquash`. It allows to\n> fixup both the content and the commit message of the specified commit.\n> Here we add two suboptions to the `--fixup`, first `amend` suboption that\n> creates an \"amend!\" commit. It takes the staged changes and also allows to\n> edit the commit message of the commit we are fixing.\n> Example usuage:\n> git commit --fixup=amend:\n>\n> Secondly, `reword` suboption that creates an empty \"amend!\" commit i.e it\n> ignores the staged changes and only allows to reword/edit the commit message\n> of the commit we are fixing.\n> Example usuage:\n> git commit --fixup=reword:\n\nWe used to have only --fixup that was meant to squeeze in minor\ncorrections to the contents recorded, and it kept the log message\nof the original commit intact.\n\nNow we have two other ways, --fixup=reword that is meant to correct\nonly the log message while keeping the contents intact from the\noriginal, and --fixup=amend that is meant to allow users to do both.\nThey are nice additions to our toolbox.\n\nWhile trying to use the --fixup=amend myself to \"touch up\" somebody\nelse's work today, another thing that we did not discuss so far came\nto my mind (sorry, if this was discussed and resolved in your\nprevious discussions with other reviewers). What should we do to\nthe authorship?\n\nFor the original --fixup, it is reasonably obvious that the original\nauthorship should be kept, as the intended use case is to make a\nsmall tweak that does not change the intention of the commit in any\nway (and that is why the log message from the original is kept), and\nwith --fixup=reword, it would probably be the same (the contents\nwere written by the original author alone, and the person fixing-up\nis not changing only the log message). So these two have a\nreasonably good default model for the authorship information for the\nfinal outcome: the original authorship should be kept (of course,\nthe user can easily take over the authorship later with \"git commit\n--amend --reset-author\" perhaps run as part of \"rebase -i\", if the\ncontribution is significant enough to deserve the transfer of the\nauthorship).\n\nBut I am not sure what the default behaviour for the authorship when\n--fixup=amend: is used to update somebody else's commit. I\nthink it is OK to leave it to whatever the code happens to do right\nnow (simply because I have no strong reason to vote for either way\nbetween keeping the original and letting the amending user take it\nover), but I think it should be documented what happens in each\ncase.\n\nThanks.\n\n\n\n"},{"id":"417665","messageId":"CAPSFM5fL3_pkMNBYSXiRHzbqOwzA0SBH3NjU+zVq3Mxv7UgL4w@mail.gmail.com","threadId":"55160","inReplyTo":"xmqqeeh61rv9.fsf@gitster.g","subject":"Re: [PATCH 0/6][Outreachy] commit: Implementation of \"amend!\" commit","fromName":"Charvi Mendiratta","fromEmail":"charvi077@gmail.com","sentAt":"2021-02-24T05:54:43Z","receivedAt":"2021-02-24T05:55:53Z","isPatch":true,"sender":{"key":"charvi077@gmail.com","avatar":"https://avatars.githubusercontent.com/u/32788121?v=4"},"body":"On Wed, 24 Feb 2021 at 01:25, Junio C Hamano wrote:\n>\n[...]\n> We used to have only --fixup that was meant to squeeze in minor\n> corrections to the contents recorded, and it kept the log message\n> of the original commit intact.\n>\n> Now we have two other ways, --fixup=reword that is meant to correct\n> only the log message while keeping the contents intact from the\n> original, and --fixup=amend that is meant to allow users to do both.\n> They are nice additions to our toolbox.\n>\n> While trying to use the --fixup=amend myself to \"touch up\" somebody\n> else's work today, another thing that we did not discuss so far came\n> to my mind (sorry, if this was discussed and resolved in your\n> previous discussions with other reviewers). What should we do to\n> the authorship?\n>\n\nYes, for the authorship similar to `--fixup`, when used with suboptions\n`amend` or `reword`, it keeps the original authorship.\n\n> For the original --fixup, it is reasonably obvious that the original\n> authorship should be kept, as the intended use case is to make a\n> small tweak that does not change the intention of the commit in any\n> way (and that is why the log message from the original is kept), and\n> with --fixup=reword, it would probably be the same (the contents\n> were written by the original author alone, and the person fixing-up\n> is not changing only the log message). So these two have a\n> reasonably good default model for the authorship information for the\n> final outcome: the original authorship should be kept (of course,\n> the user can easily take over the authorship later with \"git commit\n> --amend --reset-author\" perhaps run as part of \"rebase -i\", if the\n> contribution is significant enough to deserve the transfer of the\n> authorship).\n>\n> But I am not sure what the default behaviour for the authorship when\n> --fixup=amend: is used to update somebody else's commit. I\n> think it is OK to leave it to whatever the code happens to do right\n> now (simply because I have no strong reason to vote for either way\n> between keeping the original and letting the amending user take it\n> over), but I think it should be documented what happens in each\n> case.\n>\n\nOkay, I agree. We have included in the tests where we check both the\nresulting commit message and the author details but yes I will document\nit as well.\n\nThanks and Regards,\nCharvi\n"},{"id":"417806","messageId":"20210225100855.25530-1-charvi077@gmail.com","threadId":"55160","inReplyTo":"20210217072904.16257-1-charvi077@gmail.com","subject":"[PATCH v2 0/6][Outreachy] commit: Implementation of \"amend!\" commit","fromName":"Charvi Mendiratta","fromEmail":"charvi077@gmail.com","sentAt":"2021-02-25T10:08:52Z","receivedAt":"2021-02-25T10:16:57Z","isPatch":true,"sender":{"key":"charvi077@gmail.com","avatar":"https://avatars.githubusercontent.com/u/32788121?v=4"},"body":"This patch series teaches `git commit --fixup` to create \"amend!\" commit\nas an alternative that works with `git rebase --autosquash`. It allows to\nfixup both the content and the commit message of the specified commit.\nHere we add two suboptions to the `--fixup`, first `amend` suboption that\ncreates an \"amend!\" commit. It takes the staged changes and also allows to\nedit the commit message of the commit we are fixing.\nExample usuage:\ngit commit --fixup=amend:\n\nSecondly, `reword` suboption that creates an empty \"amend!\" commit i.e it\nignores the staged changes and only allows to reword/edit the commit message\nof the commit we are fixing.\nExample usuage:\ngit commit --fixup=reword:\n\n** This work is rebased on the top of cm/rebase-i-updates.\n\nChanges from v1:\n(Thanks Junio C Hamano for the reviews and pointing out the changes required in v1)\n\n* (As `reword` and `amend` suboptions of `--fixup` uses apha\n characters only) add helper function that does strspn(alpha) to\n detect that suboptions are passed to `--fixup` instead of checking\n if ':' occurs before '^' or '@'.\n\n* update the `check_fixup_reword_options()` function and disallow\n `reword` option with path(argc), interactive and patch_interactive.\n Also update the test script (t7500).\n\n* make '-m' commit message option incompatible with `amend` and\n `reword` suboption, as it is more expected to edit the original\n commit message while preparing \"amend!\" commit and using `-m` for\n directly appending the commit message results in the confusion at\n the user end. So, error out when combined with `m` and update\n the respective tests.\n\n* slight improvements in commit messages and update the documention\n (git-commit.txt) to include the above changes.\n\nCharvi Mendiratta (6):\n sequencer: export subject_length()\n commit: add amend suboption to --fixup to create amend! commit\n commit: add a reword suboption to --fixup\n t7500: add tests for --fixup=[amend|reword] options\n t3437: use --fixup with options to create amend! commit\n doc/git-commit: add documentation for fixup=[amend|reword] options\n\n Documentation/git-commit.txt | 39 ++++++-\n Documentation/git-rebase.txt | 21 ++--\n builtin/commit.c | 117 +++++++++++++++++--\n commit.c | 14 +++\n commit.h | 3 +\n sequencer.c | 14 ---\n t/t3437-rebase-fixup-options.sh | 30 +----\n t/t7500-commit-template-squash-signoff.sh | 134 ++++++++++++++++++++++\n 8 files changed, 306 insertions(+), 66 deletions(-)\n\n--\n2.29.0.rc1\n\n"},{"id":"417807","messageId":"20210225100855.25530-2-charvi077@gmail.com","threadId":"55160","inReplyTo":"20210217072904.16257-1-charvi077@gmail.com","subject":"[PATCH v2 1/6] sequencer: export subject_length()","fromName":"Charvi Mendiratta","fromEmail":"charvi077@gmail.com","sentAt":"2021-02-25T10:08:54Z","receivedAt":"2021-02-25T10:17:28Z","isPatch":true,"sender":{"key":"charvi077@gmail.com","avatar":"https://avatars.githubusercontent.com/u/32788121?v=4"},"body":"This function can be used in other parts of git. Let's move the\nfunction to commit.c.\n\nMentored-by: Christian Couder \nMentored-by: Phillip Wood \nSigned-off-by: Charvi Mendiratta \n---\n commit.c | 14 ++++++++++++++\n commit.h | 3 +++\n sequencer.c | 14 --------------\n 3 files changed, 17 insertions(+), 14 deletions(-)\n\ndiff --git a/commit.c b/commit.c\nindex bab8d5ab07..41cc72c4de 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -535,6 +535,20 @@ int find_commit_subject(const char *commit_buffer, const char **subject)\n \treturn eol - p;\n }\n \n+size_t subject_length(const char *body)\n+{\n+\tconst char *p = body;\n+\twhile (*p) {\n+\t\tconst char *next = skip_blank_lines(p);\n+\t\tif (next != p)\n+\t\t\tbreak;\n+\t\tp = strchrnul(p, '\\n');\n+\t\tif (*p)\n+\t\t\tp++;\n+\t}\n+\treturn p - body;\n+}\n+\n struct commit_list *commit_list_insert(struct commit *item, struct commit_list **list_p)\n {\n \tstruct commit_list *new_list = xmalloc(sizeof(struct commit_list));\ndiff --git a/commit.h b/commit.h\nindex f4e7b0158e..12ff6bc641 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -165,6 +165,9 @@ const void *detach_commit_buffer(struct commit *, unsigned long *sizep);\n /* Find beginning and length of commit subject. */\n int find_commit_subject(const char *commit_buffer, const char **subject);\n \n+/* Return length of the commit subject from commit log message. */\n+size_t subject_length(const char *body);\n+\n struct commit_list *commit_list_insert(struct commit *item,\n \t\t\t\t\tstruct commit_list **list);\n int commit_list_contains(struct commit *item,\ndiff --git a/sequencer.c b/sequencer.c\nindex abc6d5cdfd..3fac4ba62f 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -1724,20 +1724,6 @@ enum todo_item_flags {\n \tTODO_EDIT_FIXUP_MSG = (1 << 2),\n };\n \n-static size_t subject_length(const char *body)\n-{\n-\tconst char *p = body;\n-\twhile (*p) {\n-\t\tconst char *next = skip_blank_lines(p);\n-\t\tif (next != p)\n-\t\t\tbreak;\n-\t\tp = strchrnul(p, '\\n');\n-\t\tif (*p)\n-\t\t\tp++;\n-\t}\n-\treturn p - body;\n-}\n-\n static const char first_commit_msg_str[] = N_(\"This is the 1st commit message:\");\n static const char nth_commit_msg_fmt[] = N_(\"This is the commit message #%d:\");\n static const char skip_first_commit_msg_str[] = N_(\"The 1st commit message will be skipped:\");\n-- \n2.29.0.rc1\n\n"},{"id":"417808","messageId":"20210225100855.25530-3-charvi077@gmail.com","threadId":"55160","inReplyTo":"20210217072904.16257-1-charvi077@gmail.com","subject":"[PATCH v2 2/6] commit: add amend suboption to --fixup to create amend! commit","fromName":"Charvi Mendiratta","fromEmail":"charvi077@gmail.com","sentAt":"2021-02-25T10:08:56Z","receivedAt":"2021-02-25T10:19:48Z","isPatch":true,"sender":{"key":"charvi077@gmail.com","avatar":"https://avatars.githubusercontent.com/u/32788121?v=4"},"body":"`git commit --fixup=amend:` will create an \"amend!\" commit.\nThe resulting commit message subject will be \"amend! ...\" where\n\"...\" is the subject line of and the initial message\nbody will be 's message. -m can be used to override the\nmessage body.\n\nThe \"amend!\" commit when rebased with --autosquash will fixup the\ncontents and replace the commit message of with the\n\"amend!\" commit's message body.\n\nInorder to prevent rebase from creating commits with an empty\nmessage we refuse to create an \"amend!\" commit if commit message\nbody is empty.\n\nMentored-by: Christian Couder \nMentored-by: Phillip Wood \nHelped-by: Junio C Hamano \nSigned-off-by: Charvi Mendiratta \n---\n builtin/commit.c | 89 +++++++++++++++++++++++++++++++++++++++++++-----\n 1 file changed, 80 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 505fe60956..56ae15a762 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -105,7 +105,8 @@ static const char *template_file;\n */\n static const char *author_message, *author_message_buffer;\n static char *edit_message, *use_message;\n-static char *fixup_message, *squash_message;\n+static char *fixup_message, *fixup_commit, *squash_message;\n+static const char *fixup_prefix;\n static int all, also, interactive, patch_interactive, only, amend, signoff;\n static int edit_flag = -1; /* unspecified */\n static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;\n@@ -681,6 +682,21 @@ static void adjust_comment_line_char(const struct strbuf *sb)\n \tcomment_line_char = *p;\n }\n \n+static int prepare_amend_commit(struct commit *commit, struct strbuf *sb,\n+\t\t\t\t\t\t\t\tstruct pretty_print_context *ctx) {\n+\t/*\n+\t * If we amend the 'amend!' commit then we don't want to\n+\t * duplicate the subject line.\n+\t */\n+\tconst char *format = NULL;\n+\tif (starts_with(sb->buf, \"amend! amend!\"))\n+\t\tformat = \"%b\";\n+\telse\n+\t\tformat = \"%B\";\n+\tformat_commit_message(commit, format, sb, ctx);\n+\treturn 0;\n+}\n+\n static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\t\t struct commit *current_head,\n \t\t\t struct wt_status *s,\n@@ -745,15 +761,24 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t} else if (fixup_message) {\n \t\tstruct pretty_print_context ctx = {0};\n \t\tstruct commit *commit;\n-\t\tcommit = lookup_commit_reference_by_name(fixup_message);\n+\t\tchar *fmt = xstrfmt(\"%s! %%s\\n\\n\", fixup_prefix);\n+\t\tcommit = lookup_commit_reference_by_name(fixup_commit);\n \t\tif (!commit)\n-\t\t\tdie(_(\"could not lookup commit %s\"), fixup_message);\n+\t\t\tdie(_(\"could not lookup commit %s\"), fixup_commit);\n \t\tctx.output_encoding = get_commit_output_encoding();\n-\t\tformat_commit_message(commit, \"fixup! %s\\n\\n\",\n-\t\t\t\t &sb, &ctx);\n-\t\tif (have_option_m)\n-\t\t\tstrbuf_addbuf(&sb, &message);\n+\t\tformat_commit_message(commit, fmt, &sb, &ctx);\n+\t\tfree(fmt);\n \t\thook_arg1 = \"message\";\n+\n+\t\tif ((have_option_m) && !strcmp(fixup_prefix,\"fixup\"))\n+\t\t\tstrbuf_addbuf(&sb, &message);\n+\n+\t\tif (!strcmp(fixup_prefix,\"amend\")) {\n+\t\t\tif (have_option_m)\n+\t\t\t\tdie(_(\"cannot combine -m with --fixup:%s\"), fixup_message);\n+\t\t\telse\n+\t\t\t\tprepare_amend_commit(commit, &sb, &ctx);\n+\t\t}\n \t} else if (!stat(git_path_merge_msg(the_repository), &statbuf)) {\n \t\tsize_t merge_msg_start;\n \n@@ -1152,6 +1177,12 @@ static void finalize_deferred_config(struct wt_status *s)\n \t\ts->ahead_behind_flags = AHEAD_BEHIND_FULL;\n }\n \n+/* returns the length of intial segment of alpha characters only */\n+static size_t get_alpha_len(char *fixup_message) {\n+\tconst char alphas[] = \"abcdefghijklmnopqrstuvwxyz\";\n+\treturn strspn(fixup_message, alphas);\n+}\n+\n static int parse_and_validate_options(int argc, const char *argv[],\n \t\t\t\t const struct option *options,\n \t\t\t\t const char * const usage[],\n@@ -1170,7 +1201,7 @@ static int parse_and_validate_options(int argc, const char *argv[],\n \tif (force_author && renew_authorship)\n \t\tdie(_(\"Using both --reset-author and --author does not make sense\"));\n \n-\tif (logfile || have_option_m || use_message || fixup_message)\n+\tif (logfile || have_option_m || use_message)\n \t\tuse_editor = 0;\n \tif (0 <= edit_flag)\n \t\tuse_editor = edit_flag;\n@@ -1227,6 +1258,28 @@ static int parse_and_validate_options(int argc, const char *argv[],\n \n \tif (also + only + all + interactive > 1)\n \t\tdie(_(\"Only one of --include/--only/--all/--interactive/--patch can be used.\"));\n+\n+\tif (fixup_message) {\n+\t\t/*\n+\t\t * As `amend` suboption contains only alpha\n+\t\t * character. So check if first non alpha\n+\t\t * character in fixup_message is ':'.\n+\t\t */\n+\t\tsize_t len = get_alpha_len(fixup_message);\n+\t\tif (len && fixup_message[len] == ':') {\n+\t\t\tfixup_message[len] = '\\0';\n+\t\t\tfixup_commit = fixup_message + ++len;\n+\t\t\tif (starts_with(\"amend\", fixup_message))\n+\t\t\t\tfixup_prefix = \"amend\";\n+\t\t\telse\n+\t\t\t\tdie(_(\"unknown option: --fixup=%s:%s\"), fixup_message, fixup_commit);\n+\t\t} else {\n+\t\t\tfixup_commit = fixup_message;\n+\t\t\tfixup_prefix = \"fixup\";\n+\t\t\tuse_editor = 0;\n+\t\t}\n+\t}\n+\n \tcleanup_mode = get_cleanup_mode(cleanup_arg, use_editor);\n \n \thandle_untracked_files_arg(s);\n@@ -1504,7 +1557,12 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\tOPT_CALLBACK('m', \"message\", &message, N_(\"message\"), N_(\"commit message\"), opt_parse_m),\n \t\tOPT_STRING('c', \"reedit-message\", &edit_message, N_(\"commit\"), N_(\"reuse and edit message from specified commit\")),\n \t\tOPT_STRING('C', \"reuse-message\", &use_message, N_(\"commit\"), N_(\"reuse message from specified commit\")),\n-\t\tOPT_STRING(0, \"fixup\", &fixup_message, N_(\"commit\"), N_(\"use autosquash formatted message to fixup specified commit\")),\n+\t\t/*\n+\t\t * TRANSLATORS: please do not translate [amend:]\n+\t\t * Here \"amend\" is an option to the --fixup command\n+\t\t * line flag, that creates amend! commit.\n+\t\t */\n+\t\tOPT_STRING(0, \"fixup\", &fixup_message, N_(\"[amend:]commit\"), N_(\"use autosquash formatted message to fixup or amend specified commit\")),\n \t\tOPT_STRING(0, \"squash\", &squash_message, N_(\"commit\"), N_(\"use autosquash formatted message to squash specified commit\")),\n \t\tOPT_BOOL(0, \"reset-author\", &renew_authorship, N_(\"the commit is authored by me now (used with -C/-c/--amend)\")),\n \t\tOPT_BOOL('s', \"signoff\", &signoff, N_(\"add a Signed-off-by trailer\")),\n@@ -1663,6 +1721,19 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\texit(1);\n \t}\n \n+\tif (fixup_message && starts_with(sb.buf, \"amend! \") &&\n+\t\t!allow_empty_message) {\n+\t\tstruct strbuf body = STRBUF_INIT;\n+\t\tsize_t len = subject_length(sb.buf);\n+\t\tstrbuf_addstr(&body, sb.buf + len);\n+\t\tif (message_is_empty(&body, cleanup_mode)) {\n+\t\t\trollback_index_files();\n+\t\t\tfprintf(stderr, _(\"Aborting commit due to empty commit message body.\\n\"));\n+\t\t\texit(1);\n+\t\t}\n+\t\tstrbuf_release(&body);\n+\t}\n+\n \tif (amend) {\n \t\tconst char *exclude_gpgsig[3] = { \"gpgsig\", \"gpgsig-sha256\", NULL };\n \t\textra = read_commit_extra_headers(current_head, exclude_gpgsig);\n-- \n2.29.0.rc1\n\n"},{"id":"417809","messageId":"20210225100855.25530-4-charvi077@gmail.com","threadId":"55160","inReplyTo":"20210217072904.16257-1-charvi077@gmail.com","subject":"[PATCH v2 3/6] commit: add a reword suboption to --fixup","fromName":"Charvi Mendiratta","fromEmail":"charvi077@gmail.com","sentAt":"2021-02-25T10:08:58Z","receivedAt":"2021-02-25T10:20:20Z","isPatch":true,"sender":{"key":"charvi077@gmail.com","avatar":"https://avatars.githubusercontent.com/u/32788121?v=4"},"body":"`git commit --fixup=reword:` creates an empty \"amend!\" commit\nthat will reword without changing its contents when it is\nrebased with --autosquash.\n\nApart from ignoring staged changes it works similarly to\n`--fixup=amend:`.\n\nMentored-by: Christian Couder \nMentored-by: Phillip Wood \nHelped-by: Junio C Hamano \nSigned-off-by: Charvi Mendiratta \n---\n builtin/commit.c | 46 +++++++++++++++++++++++++++++++++++++---------\n 1 file changed, 37 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 56ae15a762..82e77aa61d 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1177,6 +1177,27 @@ static void finalize_deferred_config(struct wt_status *s)\n \t\ts->ahead_behind_flags = AHEAD_BEHIND_FULL;\n }\n \n+static void check_fixup_reword_options(int argc) {\n+\tif (whence != FROM_COMMIT) {\n+\t\tif (whence == FROM_MERGE)\n+\t\t\tdie(_(\"You are in the middle of a merge -- cannot reword.\"));\n+\t\telse if (is_from_cherry_pick(whence))\n+\t\t\tdie(_(\"You are in the middle of a cherry-pick -- cannot reword.\"));\n+\t}\n+\tif (argc)\n+\t\tdie(_(\"cannot combine reword option of --fixup with paths\"));\n+\tif (patch_interactive)\n+\t\tdie(_(\"cannot combine reword option of --fixup with --patch\"));\n+\tif (interactive)\n+\t\tdie(_(\"cannot combine reword option of --fixup with --interactive\"));\n+\tif (all)\n+\t\tdie(_(\"cannot combine reword option of --fixup with --all\"));\n+\tif (also)\n+\t\tdie(_(\"cannot combine reword option of --fixup with --include\"));\n+\tif (only)\n+\t\tdie(_(\"cannot combine reword option of --fixup with --only\"));\n+}\n+\n /* returns the length of intial segment of alpha characters only */\n static size_t get_alpha_len(char *fixup_message) {\n \tconst char alphas[] = \"abcdefghijklmnopqrstuvwxyz\";\n@@ -1261,18 +1282,25 @@ static int parse_and_validate_options(int argc, const char *argv[],\n \n \tif (fixup_message) {\n \t\t/*\n-\t\t * As `amend` suboption contains only alpha\n-\t\t * character. So check if first non alpha\n-\t\t * character in fixup_message is ':'.\n+\t\t * As `amend`/`reword` suboptions contains only alpha\n+\t\t * characters. So check if first non alpha character\n+\t\t * in fixup_message is ':'.\n \t\t */\n \t\tsize_t len = get_alpha_len(fixup_message);\n \t\tif (len && fixup_message[len] == ':') {\n \t\t\tfixup_message[len] = '\\0';\n \t\t\tfixup_commit = fixup_message + ++len;\n-\t\t\tif (starts_with(\"amend\", fixup_message))\n+\t\t\tif (starts_with(\"amend\", fixup_message) ||\n+\t\t\t\tstarts_with(\"reword\", fixup_message)) {\n \t\t\t\tfixup_prefix = \"amend\";\n-\t\t\telse\n+\t\t\t\tif (*fixup_message == 'r') {\n+\t\t\t\t\tcheck_fixup_reword_options(argc);\n+\t\t\t\t\tallow_empty = 1;\n+\t\t\t\t\tonly = 1;\n+\t\t\t\t}\n+\t\t\t} else {\n \t\t\t\tdie(_(\"unknown option: --fixup=%s:%s\"), fixup_message, fixup_commit);\n+\t\t\t}\n \t\t} else {\n \t\t\tfixup_commit = fixup_message;\n \t\t\tfixup_prefix = \"fixup\";\n@@ -1558,11 +1586,11 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\tOPT_STRING('c', \"reedit-message\", &edit_message, N_(\"commit\"), N_(\"reuse and edit message from specified commit\")),\n \t\tOPT_STRING('C', \"reuse-message\", &use_message, N_(\"commit\"), N_(\"reuse message from specified commit\")),\n \t\t/*\n-\t\t * TRANSLATORS: please do not translate [amend:]\n-\t\t * Here \"amend\" is an option to the --fixup command\n-\t\t * line flag, that creates amend! commit.\n+\t\t * TRANSLATORS: please do not translate [(amend|reword):]\n+\t\t * Here \"amend\" and \"reword\" are options to the --fixup\n+\t\t * command line flag, that creates amend! commit.\n \t\t */\n-\t\tOPT_STRING(0, \"fixup\", &fixup_message, N_(\"[amend:]commit\"), N_(\"use autosquash formatted message to fixup or amend specified commit\")),\n+\t\tOPT_STRING(0, \"fixup\", &fixup_message, N_(\"[(amend|reword):]commit\"), N_(\"use autosquash formatted message to fixup or amend/reword specified commit\")),\n \t\tOPT_STRING(0, \"squash\", &squash_message, N_(\"commit\"), N_(\"use autosquash formatted message to squash specified commit\")),\n \t\tOPT_BOOL(0, \"reset-author\", &renew_authorship, N_(\"the commit is authored by me now (used with -C/-c/--amend)\")),\n \t\tOPT_BOOL('s', \"signoff\", &signoff, N_(\"add a Signed-off-by trailer\")),\n-- \n2.29.0.rc1\n\n"},{"id":"417810","messageId":"20210225100855.25530-6-charvi077@gmail.com","threadId":"55160","inReplyTo":"20210217072904.16257-1-charvi077@gmail.com","subject":"[PATCH v2 5/6] t3437: use --fixup with options to create amend! commit","fromName":"Charvi Mendiratta","fromEmail":"charvi077@gmail.com","sentAt":"2021-02-25T10:09:02Z","receivedAt":"2021-02-25T10:21:54Z","isPatch":true,"sender":{"key":"charvi077@gmail.com","avatar":"https://avatars.githubusercontent.com/u/32788121?v=4"},"body":"We taught `git commit --fixup` to create \"amend!\" commit. Let's also\nupdate the tests and use it to setup the rebase tests.\n\nMentored-by: Christian Couder \nMentored-by: Phillip Wood \nSigned-off-by: Charvi Mendiratta \n---\n t/t3437-rebase-fixup-options.sh | 30 +++---------------------------\n 1 file changed, 3 insertions(+), 27 deletions(-)\n\ndiff --git a/t/t3437-rebase-fixup-options.sh b/t/t3437-rebase-fixup-options.sh\nindex a5a20354e3..d0bdc7ed02 100755\n--- a/t/t3437-rebase-fixup-options.sh\n+++ b/t/t3437-rebase-fixup-options.sh\n@@ -72,40 +72,16 @@ test_expect_success 'setup' '\n \tgit commit --fixup=HEAD -a &&\n \tgit tag B1 &&\n \ttest_tick &&\n-\tgit commit --allow-empty -F - <<-EOF &&\n-\tamend! B\n-\t$EMPTY\n-\tB\n-\t$EMPTY\n-\tedited 1\n-\tEOF\n+\tFAKE_COMMIT_AMEND=\"edited 1\" git commit --fixup=reword:B &&\n \ttest_tick &&\n-\tgit commit --allow-empty -F - <<-EOF &&\n-\tamend! amend! B\n-\t$EMPTY\n-\tB\n-\t$EMPTY\n-\tedited 1\n-\t$EMPTY\n-\tedited 2\n-\tEOF\n+\tFAKE_COMMIT_AMEND=\"edited 2\" git commit --fixup=reword:HEAD &&\n \techo B2 >B &&\n \ttest_tick &&\n \tFAKE_COMMIT_AMEND=\"edited squash\" git commit --squash=HEAD -a &&\n \tgit tag B2 &&\n \techo B3 >B &&\n \ttest_tick &&\n-\tgit commit -a -F - <<-EOF &&\n-\tamend! amend! amend! B\n-\t$EMPTY\n-\tB\n-\t$EMPTY\n-\tedited 1\n-\t$EMPTY\n-\tedited 2\n-\t$EMPTY\n-\tedited 3\n-\tEOF\n+\tFAKE_COMMIT_AMEND=\"edited 3\" git commit -a --fixup=amend:HEAD^ &&\n \tgit tag B3 &&\n \n \tGIT_AUTHOR_NAME=\"Rebase Author\" &&\n-- \n2.29.0.rc1\n\n"},{"id":"417811","messageId":"20210225100855.25530-5-charvi077@gmail.com","threadId":"55160","inReplyTo":"20210217072904.16257-1-charvi077@gmail.com","subject":"[PATCH v2 4/6] t7500: add tests for --fixup=[amend|reword] options","fromName":"Charvi Mendiratta","fromEmail":"charvi077@gmail.com","sentAt":"2021-02-25T10:09:00Z","receivedAt":"2021-02-25T10:22:59Z","isPatch":true,"sender":{"key":"charvi077@gmail.com","avatar":"https://avatars.githubusercontent.com/u/32788121?v=4"},"body":"Mentored-by: Christian Couder \nMentored-by: Phillip Wood \nSigned-off-by: Charvi Mendiratta \n---\n t/t7500-commit-template-squash-signoff.sh | 135 ++++++++++++++++++++++\n 1 file changed, 135 insertions(+)\n\ndiff --git a/t/t7500-commit-template-squash-signoff.sh b/t/t7500-commit-template-squash-signoff.sh\nindex 6d19ece05d..3b3a39fcaf 100755\n--- a/t/t7500-commit-template-squash-signoff.sh\n+++ b/t/t7500-commit-template-squash-signoff.sh\n@@ -9,6 +9,8 @@ Tests for template, signoff, squash and -F functions.'\n \n . ./test-lib.sh\n \n+. \"$TEST_DIRECTORY\"/lib-rebase.sh\n+\n commit_msg_is () {\n \texpect=commit_msg_is.expect\n \tactual=commit_msg_is.actual\n@@ -279,6 +281,139 @@ test_expect_success 'commit --fixup -m\"something\" -m\"extra\"' '\n \n extra\"\n '\n+get_commit_msg () {\n+\trev=\"$1\" &&\n+\tgit log -1 --pretty=format:\"%B\" \"$rev\"\n+}\n+\n+test_expect_success 'commit --fixup=amend: creates amend! commit' '\n+\tcommit_for_rebase_autosquash_setup &&\n+\tcat >expected <<-EOF &&\n+\tamend! $(git log -1 --format=%s HEAD~)\n+\n+\t$(get_commit_msg HEAD~)\n+\n+\tedited\n+\tEOF\n+\t(\n+\t\tset_fake_editor &&\n+\t\tFAKE_COMMIT_AMEND=\"edited\" \\\n+\t\t\tgit commit --fixup=amend:HEAD~\n+\t) &&\n+\tget_commit_msg HEAD >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success '--fixup=reword: does not commit staged changes' '\n+\tcommit_for_rebase_autosquash_setup &&\n+\tcat >expected <<-EOF &&\n+\tamend! $(git log -1 --format=%s HEAD~)\n+\n+\t$(get_commit_msg HEAD~)\n+\n+\tedited\n+\tEOF\n+\t(\n+\t\tset_fake_editor &&\n+\t\tFAKE_COMMIT_AMEND=\"edited\" \\\n+\t\t\tgit commit --fixup=reword:HEAD~\n+\t) &&\n+\tget_commit_msg HEAD >actual &&\n+\ttest_cmp expected actual &&\n+\ttest_cmp_rev HEAD@{1}^{tree} HEAD^{tree} &&\n+\ttest_cmp_rev HEAD@{1} HEAD^ &&\n+\ttest_expect_code 1 git diff --cached --exit-code &&\n+\tgit cat-file blob :foo >actual &&\n+\ttest_cmp foo actual\n+'\n+\n+test_expect_success '--fixup=reword: error out with -m option' '\n+\tcommit_for_rebase_autosquash_setup &&\n+\techo \"fatal: cannot combine -m with --fixup:reword\" >expect &&\n+\ttest_must_fail git commit --fixup=reword:HEAD~ -m \"reword commit message\" 2>actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '--fixup=amend: error out with -m option' '\n+\tcommit_for_rebase_autosquash_setup &&\n+\techo \"fatal: cannot combine -m with --fixup:amend\" >expect &&\n+\ttest_must_fail git commit --fixup=amend:HEAD~ -m \"amend commit message\" 2>actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'consecutive amend! commits remove amend! line from commit msg body' '\n+\tcommit_for_rebase_autosquash_setup &&\n+\tcat >expected <<-EOF &&\n+\tamend! amend! $(git log -1 --format=%s HEAD~)\n+\n+\t$(get_commit_msg HEAD~)\n+\n+\tedited 1\n+\n+\tedited 2\n+\tEOF\n+\techo \"reword new commit message\" >actual &&\n+\t(\n+\t\tset_fake_editor &&\n+\t\tFAKE_COMMIT_AMEND=\"edited 1\" \\\n+\t\t\tgit commit --fixup=reword:HEAD~ &&\n+\t\tFAKE_COMMIT_AMEND=\"edited 2\" \\\n+\t\t\tgit commit --fixup=reword:HEAD\n+\t) &&\n+\tget_commit_msg HEAD >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'deny to create amend! commit if its commit msg body is empty' '\n+\tcommit_for_rebase_autosquash_setup &&\n+\techo \"Aborting commit due to empty commit message body.\" >expected &&\n+\t(\n+\t\tset_fake_editor &&\n+\t\ttest_must_fail env FAKE_COMMIT_MESSAGE=\"amend! target message subject line\" \\\n+\t\t\tgit commit --fixup=amend:HEAD~ 2>actual\n+\t) &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'amend! commit allows empty commit msg body with --allow-empty-message' '\n+\tcommit_for_rebase_autosquash_setup &&\n+\tcat >expected <<-EOF &&\n+\tamend! $(git log -1 --format=%s HEAD~)\n+\tEOF\n+\t(\n+\t\tset_fake_editor &&\n+\t\tFAKE_COMMIT_MESSAGE=\"amend! target message subject line\" \\\n+\t\t\tgit commit --fixup=amend:HEAD~ --allow-empty-message &&\n+\t\tget_commit_msg HEAD >actual\n+\t) &&\n+\ttest_cmp expected actual\n+'\n+\n+test_fixup_reword_opt () {\n+\ttest_expect_success C_LOCALE_OUTPUT \"--fixup=reword: incompatible with $1\" \"\n+\t\techo 'fatal: cannot combine reword option of --fixup with $1' >expect &&\n+\t\ttest_must_fail git commit --fixup=reword:HEAD~ $1 2>actual &&\n+\t\ttest_cmp expect actual\n+\t\"\n+}\n+\n+for opt in --all --include --only --interactive --patch\n+do\n+\ttest_fixup_reword_opt $opt\n+done\n+\n+test_expect_success '--fixup=reword: give error with pathsec' '\n+\tcommit_for_rebase_autosquash_setup &&\n+\techo \"fatal: cannot combine reword option of --fixup with paths\" >expect &&\n+\ttest_must_fail git commit --fixup=reword:HEAD~ -- foo 2>actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '--fixup=reword: -F give error message' '\n+\techo \"fatal: Only one of -c/-C/-F/--fixup can be used.\" >expect &&\n+\ttest_must_fail git commit --fixup=reword:HEAD~ -F msg 2>actual &&\n+\ttest_cmp expect actual\n+'\n \n test_expect_success 'commit --squash works with -F' '\n \tcommit_for_rebase_autosquash_setup &&\n-- \n2.29.0.rc1\n\n"},{"id":"417812","messageId":"20210225100855.25530-7-charvi077@gmail.com","threadId":"55160","inReplyTo":"20210217072904.16257-1-charvi077@gmail.com","subject":"[PATCH v2 6/6] doc/git-commit: add documentation for fixup=[amend|reword] options","fromName":"Charvi Mendiratta","fromEmail":"charvi077@gmail.com","sentAt":"2021-02-25T10:09:04Z","receivedAt":"2021-02-25T10:23:32Z","isPatch":true,"sender":{"key":"charvi077@gmail.com","avatar":"https://avatars.githubusercontent.com/u/32788121?v=4"},"body":"Mentored-by: Christian Couder \nMentored-by: Phillip Wood \nHelped-by: Junio C Hamano \nSigned-off-by: Charvi Mendiratta \n---\n Documentation/git-commit.txt | 39 ++++++++++++++++++++++++++++++------\n Documentation/git-rebase.txt | 21 ++++++++++---------\n 2 files changed, 44 insertions(+), 16 deletions(-)\n\ndiff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt\nindex 17150fa7ea..bc50301b1c 100644\n--- a/Documentation/git-commit.txt\n+++ b/Documentation/git-commit.txt\n@@ -9,7 +9,7 @@ SYNOPSIS\n --------\n [verse]\n 'git commit' [-a | --interactive | --patch] [-s] [-v] [-u] [--amend]\n-\t [--dry-run] [(-c | -C | --fixup | --squash) ]\n+\t [--dry-run] [(-c | -C | --squash) | --fixup [(amend|reword):])]\n \t [-F | -m ] [--reset-author] [--allow-empty]\n \t [--allow-empty-message] [--no-verify] [-e] [--author=]\n \t [--date=] [--cleanup=] [--[no-]status]\n@@ -86,11 +86,38 @@ OPTIONS\n \tLike '-C', but with `-c` the editor is invoked, so that\n \tthe user can further edit the commit message.\n \n---fixup=::\n-\tConstruct a commit message for use with `rebase --autosquash`.\n-\tThe commit message will be the subject line from the specified\n-\tcommit with a prefix of \"fixup! \". See linkgit:git-rebase[1]\n-\tfor details.\n+--fixup=[(amend|reword):]::\n+\tWhen used without options, lets's say `git commit --fixup=`,\n+\tit creates a \"fixup!\" commit where the commit message will be\n+\tthe subject line from the specified commit with a prefix of\n+\t\"fixup! \". The resulting \"fixup!\" commit is further used with\n+\t`git rebase --autosquash` to fixup the content of the specified\n+\tcommit.\n++\n+When used with option `amend`, let's say `git commit --fixup=amend:`,\n+it creates a \"amend!\" commit to fixup both the content and the commit log\n+message of the specified commit. The resulting \"amend!\" commit's commit\n+message subject will be the subject line from the specified commit with a\n+prefix of \"amend! \" and the message body will be commit log message of the\n+specified commit. It also invokes an editor seeded with the \"amend!\" commit\n+log message to allow to edit further. And it denies to create \"amend!\" commit\n+if it's commit message body is empty unless used with `allow-empty-message`\n+option. \"amend!\" commit when rebased with `--autosquash` will fixup the\n+contents and replace the commit message of the specified commit with the\n+\"amend!\" commit's message body.\n++\n+When used with alternative option `reword`, let's say\n+`git commit --fixup=reword:`, it works similar to `amend` option, but\n+here it creates an empty \"amend!\" commit, i.e it does not take any staged\n+changes and only allows to fixup the commit message of the specified commit.\n+It will reword the specified commit when it is rebased with `--autosquash`.\n++\n+Unlike `--fixup` without options, `--fixup=[amend/reword]:` is incompatible with\n+`-m` commit message option.\n++\n+Also, after fixing the commit using `--fixup`, with or without option and rebased\n+with `--autosquash`, the authorship of the original commit remains unchanged. See\n+linkgit:git-rebase[1] for details.\n \n --squash=::\n \tConstruct a commit message for use with `rebase --autosquash`.\ndiff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt\nindex 8bfa5a9272..ffea76e53b 100644\n--- a/Documentation/git-rebase.txt\n+++ b/Documentation/git-rebase.txt\n@@ -593,16 +593,17 @@ See also INCOMPATIBLE OPTIONS below.\n \n --autosquash::\n --no-autosquash::\n-\tWhen the commit log message begins with \"squash! ...\" (or\n-\t\"fixup! ...\"), and there is already a commit in the todo list that\n-\tmatches the same `...`, automatically modify the todo list of rebase\n-\t-i so that the commit marked for squashing comes right after the\n-\tcommit to be modified, and change the action of the moved commit\n-\tfrom `pick` to `squash` (or `fixup`). A commit matches the `...` if\n-\tthe commit subject matches, or if the `...` refers to the commit's\n-\thash. As a fall-back, partial matches of the commit subject work,\n-\ttoo. The recommended way to create fixup/squash commits is by using\n-\tthe `--fixup`/`--squash` options of linkgit:git-commit[1].\n+\tWhen the commit log message begins with \"squash! ...\" (or \"fixup! ...\"\n+\tor \"amend! ...\"), and there is already a commit in the todo list that\n+\tmatches the same `...`, automatically modify the todo list of\n+\t`rebase -i`, so that the commit marked for squashing comes right after\n+\tthe commit to be modified, and change the action of the moved commit\n+\tfrom `pick` to `squash` (or `fixup` or `fixup -C`) respectively. A commit\n+\tmatches the `...` if the commit subject matches, or if the `...` refers\n+\tto the commit's hash. As a fall-back, partial matches of the commit\n+\tsubject work, too. The recommended way to create fixup/squash/amend\n+\tcommits is by using the `--fixup=[amend|reword]`/`--squash` options of\n+\tlinkgit:git-commit[1].\n +\n If the `--autosquash` option is enabled by default using the\n configuration variable `rebase.autoSquash`, this option can be\n-- \n2.29.0.rc1\n\n"},{"id":"417847","messageId":"xmqq7dmvubus.fsf@gitster.g","threadId":"55160","inReplyTo":"20210225100855.25530-4-charvi077@gmail.com","subject":"Re: [PATCH v2 3/6] commit: add a reword suboption to --fixup","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-25T20:32:59Z","receivedAt":"2021-02-25T20:36:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Charvi Mendiratta writes:\n\n> `git commit --fixup=reword:` creates an empty \"amend!\" commit\n> that will reword without changing its contents when it is\n> rebased with --autosquash.\n>\n> Apart from ignoring staged changes it works similarly to\n> `--fixup=amend:`.\n>\n> Mentored-by: Christian Couder \n> Mentored-by: Phillip Wood \n> Helped-by: Junio C Hamano \n> Signed-off-by: Charvi Mendiratta \n> ---\n> builtin/commit.c | 46 +++++++++++++++++++++++++++++++++++++---------\n> 1 file changed, 37 insertions(+), 9 deletions(-)\n>\n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index 56ae15a762..82e77aa61d 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -1177,6 +1177,27 @@ static void finalize_deferred_config(struct wt_status *s)\n> \t\ts->ahead_behind_flags = AHEAD_BEHIND_FULL;\n> }\n> \n> +static void check_fixup_reword_options(int argc) {\n> +\tif (whence != FROM_COMMIT) {\n> +\t\tif (whence == FROM_MERGE)\n> +\t\t\tdie(_(\"You are in the middle of a merge -- cannot reword.\"));\n> +\t\telse if (is_from_cherry_pick(whence))\n> +\t\t\tdie(_(\"You are in the middle of a cherry-pick -- cannot reword.\"));\n> +\t}\n> +\tif (argc)\n> +\t\tdie(_(\"cannot combine reword option of --fixup with paths\"));\n\nIt would be easier if the user is told \"foo\" in the message when\n\n $ git commit --fixup=reword:HEAD~ -- foo\n\n(from your tests) is attempted, no?\n\n> +\tif (patch_interactive)\n> +\t\tdie(_(\"cannot combine reword option of --fixup with --patch\"));\n> +\tif (interactive)\n> +\t\tdie(_(\"cannot combine reword option of --fixup with --interactive\"));\n> +\tif (all)\n> +\t\tdie(_(\"cannot combine reword option of --fixup with --all\"));\n> +\tif (also)\n> +\t\tdie(_(\"cannot combine reword option of --fixup with --include\"));\n> +\tif (only)\n> +\t\tdie(_(\"cannot combine reword option of --fixup with --only\"));\n> +}\n> +\n> /* returns the length of intial segment of alpha characters only */\n> static size_t get_alpha_len(char *fixup_message) {\n> \tconst char alphas[] = \"abcdefghijklmnopqrstuvwxyz\";\n> @@ -1261,18 +1282,25 @@ static int parse_and_validate_options(int argc, const char *argv[],\n> \n> \tif (fixup_message) {\n> \t\t/*\n> -\t\t * As `amend` suboption contains only alpha\n> -\t\t * character. So check if first non alpha\n> -\t\t * character in fixup_message is ':'.\n> +\t\t * As `amend`/`reword` suboptions contains only alpha\n> +\t\t * characters. So check if first non alpha character\n> +\t\t * in fixup_message is ':'.\n> \t\t */\n> \t\tsize_t len = get_alpha_len(fixup_message);\n> \t\tif (len && fixup_message[len] == ':') {\n> \t\t\tfixup_message[len] = '\\0';\n> \t\t\tfixup_commit = fixup_message + ++len;\n> -\t\t\tif (starts_with(\"amend\", fixup_message))\n> +\t\t\tif (starts_with(\"amend\", fixup_message) ||\n> +\t\t\t\tstarts_with(\"reword\", fixup_message)) {\n> \t\t\t\tfixup_prefix = \"amend\";\n> -\t\t\telse\n> +\t\t\t\tif (*fixup_message == 'r') {\n\n> +\t\t\t\t\tcheck_fixup_reword_options(argc);\n> +\t\t\t\t\tallow_empty = 1;\n> +\t\t\t\t\tonly = 1;\n\nOK. We make sure that there is no pathspec and then by giving\nonly==1, we ignore any difference that already may exist between\nHEAD and the index. IOW\n\n\tedit somefile\n\tgit add somefile\n\tgit commit --fixup=reword:\n\nwill still record the same tree as HEAD, without affected by the\naddition of somefile to the index that is done before.\n\nGood.\n\n> +\t\t\t\t}\n> +\t\t\t} else {\n> \t\t\t\tdie(_(\"unknown option: --fixup=%s:%s\"), fixup_message, fixup_commit);\n> +\t\t\t}\n> \t\t} else {\n> \t\t\tfixup_commit = fixup_message;\n> \t\t\tfixup_prefix = \"fixup\";\n> @@ -1558,11 +1586,11 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n> \t\tOPT_STRING('c', \"reedit-message\", &edit_message, N_(\"commit\"), N_(\"reuse and edit message from specified commit\")),\n> \t\tOPT_STRING('C', \"reuse-message\", &use_message, N_(\"commit\"), N_(\"reuse message from specified commit\")),\n> \t\t/*\n> -\t\t * TRANSLATORS: please do not translate [amend:]\n> -\t\t * Here \"amend\" is an option to the --fixup command\n> -\t\t * line flag, that creates amend! commit.\n> +\t\t * TRANSLATORS: please do not translate [(amend|reword):]\n> +\t\t * Here \"amend\" and \"reword\" are options to the --fixup\n> +\t\t * command line flag, that creates amend! commit.\n\nI am not sure this comment is all that helpful to the translaters.\nIf they are not allowed to translate part, telling\nthem what that part means does not help them very much.\n\n\tLeave \"[(amend|reword):]\" as-is, and only translate .\n\nwould be more direct without distracting them with useless piece of\ninformation, no?\n\n> \t\t */\n> -\t\tOPT_STRING(0, \"fixup\", &fixup_message, N_(\"[amend:]commit\"), N_(\"use autosquash formatted message to fixup or amend specified commit\")),\n> +\t\tOPT_STRING(0, \"fixup\", &fixup_message, N_(\"[(amend|reword):]commit\"), N_(\"use autosquash formatted message to fixup or amend/reword specified commit\")),\n> \t\tOPT_STRING(0, \"squash\", &squash_message, N_(\"commit\"), N_(\"use autosquash formatted message to squash specified commit\")),\n> \t\tOPT_BOOL(0, \"reset-author\", &renew_authorship, N_(\"the commit is authored by me now (used with -C/-c/--amend)\")),\n> \t\tOPT_BOOL('s', \"signoff\", &signoff, N_(\"add a Signed-off-by trailer\")),\n"},{"id":"417848","messageId":"xmqq4khzub4m.fsf@gitster.g","threadId":"55160","inReplyTo":"20210225100855.25530-7-charvi077@gmail.com","subject":"Re: [PATCH v2 6/6] doc/git-commit: add documentation for fixup=[amend|reword] options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-25T20:48:41Z","receivedAt":"2021-02-25T20:51:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Charvi Mendiratta writes:\n\n> +--fixup=[(amend|reword):]::\n> +\tWhen used without options, lets's say `git commit --fixup=`,\n> +\tit creates a \"fixup!\" commit where the commit message will be\n\nBe careful to use word 'option' that refers to something other than\nwhat a casual reader would think of an `--option` in the description\nof the `--fixup` option.\n\n\tWithout `amend:` or `reword:`, create a `fixup!` commit where...\n\nwould be sufficient, no?\n\n> +\tthe subject line from the specified commit with a prefix of\n> +\t\"fixup! \". The resulting \"fixup!\" commit is further used with\n> +\t`git rebase --autosquash` to fixup the content of the specified\n> +\tcommit.\n> ++\n> +When used with option `amend`, let's say `git commit --fixup=amend:`,\n> +it creates a \"amend!\" commit to fixup both the content and the commit log\n\n\tThe `--fixup=amend:` form creates an \"amend!\" commit to...\n\n> +message of the specified commit. The resulting \"amend!\" commit's commit\n> +message subject will be the subject line from the specified commit with a\n> +prefix of \"amend! \" and the message body will be commit log message of the\n\nWhile that SP inside the double-quote may be technically more\ncorrect (and it was inherited from the original), I think with 'a\nprefix of \"amend!\"' is still understandable and a lot easier to\nread, especially because you'd mention \"amend!\" a few more times in\nthe same paragraph below.\n\nThe same comment applies to \"fixup! \" above.\n\n> +specified commit. It also invokes an editor seeded with the \"amend!\" commit\n> +log message to allow to edit further. And it denies to create \"amend!\" commit\n\n\"amend!\" commit log message -> log message of the \"amend!\" commit\n\ndenies -> refuses\n\n> +if it's commit message body is empty unless used with `allow-empty-message`\n> +option. \"amend!\" commit when rebased with `--autosquash` will fixup the\n\nwith the `--allow-empty-message` option.\n\n> +contents and replace the commit message of the specified commit with the\n> +\"amend!\" commit's message body.\n> ++\n> +When used with alternative option `reword`, let's say\n> +`git commit --fixup=reword:`, it works similar to `amend` option, but\n> +here it creates an empty \"amend!\" commit, i.e it does not take any staged\n\n\tThe `--fixup=reword:` form creates an `amend!`\n\tcommit similar to `--fixup=amend:` creates, but it\n\trecords the same tree as `HEAD`, i.e. it does not ...\n\n> +changes and only allows to fixup the commit message of the specified commit.\n> +It will reword the specified commit when it is rebased with `--autosquash`.\n> ++\n> +Unlike `--fixup` without options, `--fixup=[amend/reword]:` is incompatible with\n> +`-m` commit message option.\n\n\tThe `--fixup=amend:` and `--fixup=reword:` forms cannot be\n\tused with other options to add to the commit log message,\n\te.g. `-m`.\n\nAgain, why is `-m` so special? Shouldn't -F/-c/-C also be\nincompatible?\n\n> +Also, after fixing the commit using `--fixup`, with or without option and rebased\n> +with `--autosquash`, the authorship of the original commit remains unchanged. See\n> +linkgit:git-rebase[1] for details.\n\nGood.\n"},{"id":"417850","messageId":"xmqqwnuvsw0d.fsf@gitster.g","threadId":"55160","inReplyTo":"20210225100855.25530-3-charvi077@gmail.com","subject":"Re: [PATCH v2 2/6] commit: add amend suboption to --fixup to create amend! commit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-25T21:00:34Z","receivedAt":"2021-02-25T21:05:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Charvi Mendiratta writes:\n\n> `git commit --fixup=amend:` will create an \"amend!\" commit.\n> The resulting commit message subject will be \"amend! ...\" where\n> \"...\" is the subject line of and the initial message\n> body will be 's message. -m can be used to override the\n> message body.\n>\n> The \"amend!\" commit when rebased with --autosquash will fixup the\n> contents and replace the commit message of with the\n> \"amend!\" commit's message body.\n>\n> Inorder to prevent rebase from creating commits with an empty\n\nIn order to prevent ...\n\n> message we refuse to create an \"amend!\" commit if commit message\n> body is empty.\n> ...\n> +static int prepare_amend_commit(struct commit *commit, struct strbuf *sb,\n> +\t\t\t\t\t\t\t\tstruct pretty_print_context *ctx) {\n\nDon't indent the second line unnecessarily too deep.\n\n> +\t/*\n> +\t * If we amend the 'amend!' commit then we don't want to\n> +\t * duplicate the subject line.\n> +\t */\n> +\tconst char *format = NULL;\n> +\tif (starts_with(sb->buf, \"amend! amend!\"))\n> +\t\tformat = \"%b\";\n> +\telse\n> +\t\tformat = \"%B\";\n\nI am not sure how well this strategy of special case only two amend!\nscales. What would happen when we --fixup another \"fixup!\"commit?\n\nShouldn't the caller, when it called format_commit_message() to\nprepare sb it passed to us, have stripped out existing prefix, if\nany, so that we can always use the same %B format, or something like\nthat?\n\n> ...\n> +\t\tformat_commit_message(commit, fmt, &sb, &ctx);\n> +\t\tfree(fmt);\n> \t\thook_arg1 = \"message\";\n> +\n> +\t\tif ((have_option_m) && !strcmp(fixup_prefix,\"fixup\"))\n\nUnnecessary () around have_option_m, and missing SP after \",\".\n\n> +\t\t\tstrbuf_addbuf(&sb, &message);\n> +\n> +\t\tif (!strcmp(fixup_prefix,\"amend\")) {\n\nMissing SP after \",\" (I won't repeat---please re-check the whole\npatch series before rerolling).\n\n> +\t\t\tif (have_option_m)\n> +\t\t\t\tdie(_(\"cannot combine -m with --fixup:%s\"), fixup_message);\n> +\t\t\telse\n> +\t\t\t\tprepare_amend_commit(commit, &sb, &ctx);\n\nHmph, why is -m so special? Should we allow --fixup=amend:\nwith -F (or -c/-C for that matter), or are these other options\ncaught at a lot higher layer already and we do not have to check\nthem here?\n\n> \tif (also + only + all + interactive > 1)\n> \t\tdie(_(\"Only one of --include/--only/--all/--interactive/--patch can be used.\"));\n> +\n> +\tif (fixup_message) {\n> +\t\t/*\n> +\t\t * As `amend` suboption contains only alpha\n> +\t\t * character. So check if first non alpha\n> +\t\t * character in fixup_message is ':'.\n> +\t\t */\n> +\t\tsize_t len = get_alpha_len(fixup_message);\n> +\t\tif (len && fixup_message[len] == ':') {\n> +\t\t\tfixup_message[len] = '\\0';\n> +\t\t\tfixup_commit = fixup_message + ++len;\n\nIt would be easier to follow to write it this way, no?\n\n\t\t\tfixup_message[len++] = '\\0';\n\t\t\tfixup_commit = fixup_message + len;\n\n> +\t\t\tif (starts_with(\"amend\", fixup_message))\n> +\t\t\t\tfixup_prefix = \"amend\";\n> +\t\t\telse\n> +\t\t\t\tdie(_(\"unknown option: --fixup=%s:%s\"), fixup_message, fixup_commit);\n> +\t\t} else {\n> +\t\t\tfixup_commit = fixup_message;\n> +\t\t\tfixup_prefix = \"fixup\";\n> +\t\t\tuse_editor = 0;\n> +\t\t}\n> +\t}\n"},{"id":"417887","messageId":"CAPSFM5d3M5joEBmAkY_-7hYGyz_e7HVpWmR6EN2U2Hw7V_LEqQ@mail.gmail.com","threadId":"55160","inReplyTo":"xmqq7dmvubus.fsf@gitster.g","subject":"Re: [PATCH v2 3/6] commit: add a reword suboption to --fixup","fromName":"Charvi Mendiratta","fromEmail":"charvi077@gmail.com","sentAt":"2021-02-26T10:35:48Z","receivedAt":"2021-02-26T10:38:42Z","isPatch":true,"sender":{"key":"charvi077@gmail.com","avatar":"https://avatars.githubusercontent.com/u/32788121?v=4"},"body":"Hi Junio,\n\nOn Fri, 26 Feb 2021 at 02:03, Junio C Hamano wrote:\n>\n[...]\n> > + if (argc)\n> > + die(_(\"cannot combine reword option of --fixup with paths\"));\n>\n> It would be easier if the user is told \"foo\" in the message when\n>\n> $ git commit --fixup=reword:HEAD~ -- foo\n>\n> (from your tests) is attempted, no?\n>\n\nOkay, will print the passed argument in the message.\n\n>\n> I am not sure this comment is all that helpful to the translaters.\n> If they are not allowed to translate part, telling\n> them what that part means does not help them very much.\n>\n> Leave \"[(amend|reword):]\" as-is, and only translate .\n>\n> would be more direct without distracting them with useless piece of\n> information, no?\n\nYes, I agree and will correct it.\n"},{"id":"417888","messageId":"CAPSFM5deG5Z0RSPZe2rf1qs9BS8j2_84ECybiOf_LA3yLR-r-g@mail.gmail.com","threadId":"55160","inReplyTo":"xmqq4khzub4m.fsf@gitster.g","subject":"Re: [PATCH v2 6/6] doc/git-commit: add documentation for fixup=[amend|reword] options","fromName":"Charvi Mendiratta","fromEmail":"charvi077@gmail.com","sentAt":"2021-02-26T10:36:31Z","receivedAt":"2021-02-26T10:39:20Z","isPatch":true,"sender":{"key":"charvi077@gmail.com","avatar":"https://avatars.githubusercontent.com/u/32788121?v=4"},"body":"On Fri, 26 Feb 2021 at 02:18, Junio C Hamano wrote:\n>\n> Charvi Mendiratta writes:\n>\n> > +--fixup=[(amend|reword):]::\n> > + When used without options, lets's say `git commit --fixup=`,\n> > + it creates a \"fixup!\" commit where the commit message will be\n>\n> Be careful to use word 'option' that refers to something other than\n> what a casual reader would think of an `--option` in the description\n> of the `--fixup` option.\n>\n> Without `amend:` or `reword:`, create a `fixup!` commit where...\n>\n> would be sufficient, no?\n>\n\nOkay, I agree this is more clear.\n\n> > + the subject line from the specified commit with a prefix of\n> > + \"fixup! \". The resulting \"fixup!\" commit is further used with\n> > + `git rebase --autosquash` to fixup the content of the specified\n> > + commit.\n> > ++\n> > +When used with option `amend`, let's say `git commit --fixup=amend:`,\n> > +it creates a \"amend!\" commit to fixup both the content and the commit log\n>\n> The `--fixup=amend:` form creates an \"amend!\" commit to...\n>\n> > +message of the specified commit. The resulting \"amend!\" commit's commit\n> > +message subject will be the subject line from the specified commit with a\n> > +prefix of \"amend! \" and the message body will be commit log message of the\n>\n> While that SP inside the double-quote may be technically more\n> correct (and it was inherited from the original), I think with 'a\n> prefix of \"amend!\"' is still understandable and a lot easier to\n> read, especially because you'd mention \"amend!\" a few more times in\n> the same paragraph below.\n>\n> The same comment applies to \"fixup! \" above.\n>\n\nOkay, I will fix it.\n\n\n> > +specified commit. It also invokes an editor seeded with the \"amend!\" commit\n> > +log message to allow to edit further. And it denies to create \"amend!\" commit\n>\n> \"amend!\" commit log message -> log message of the \"amend!\" commit\n>\n> denies -> refuses\n>\n> > +if it's commit message body is empty unless used with `allow-empty-message`\n> > +option. \"amend!\" commit when rebased with `--autosquash` will fixup the\n>\n> with the `--allow-empty-message` option.\n>\n> > +contents and replace the commit message of the specified commit with the\n> > +\"amend!\" commit's message body.\n> > ++\n> > +When used with alternative option `reword`, let's say\n> > +`git commit --fixup=reword:`, it works similar to `amend` option, but\n> > +here it creates an empty \"amend!\" commit, i.e it does not take any staged\n>\n> The `--fixup=reword:` form creates an `amend!`\n> commit similar to `--fixup=amend:` creates, but it\n> records the same tree as `HEAD`, i.e. it does not ...\n>\n\nThanks for pointing out these changes. I agree these are more clear\nand will fix them all.\n\n> > +changes and only allows to fixup the commit message of the specified commit.\n> > +It will reword the specified commit when it is rebased with `--autosquash`.\n> > ++\n> > +Unlike `--fixup` without options, `--fixup=[amend/reword]:` is incompatible with\n> > +`-m` commit message option.\n>\n> The `--fixup=amend:` and `--fixup=reword:` forms cannot be\n> used with other options to add to the commit log message,\n> e.g. `-m`.\n>\n> Again, why is `-m` so special? Shouldn't -F/-c/-C also be\n> incompatible?\n>\n\nYes, they are also incompatible. I thought to highlight -m because `--fixup`\nallows the `-m` but I agree to reword it in above way with slight change :\ns/e.g. `-m`/i.e it is incompatible with `-m`/`-F`/`-c`/`-C` options.\n"},{"id":"417889","messageId":"CAPSFM5eKnqOhDrZWEd-c_sDNMbQkqtMwpFM0zT-gOVHvMtcZnQ@mail.gmail.com","threadId":"55160","inReplyTo":"xmqqwnuvsw0d.fsf@gitster.g","subject":"Re: [PATCH v2 2/6] commit: add amend suboption to --fixup to create amend! commit","fromName":"Charvi Mendiratta","fromEmail":"charvi077@gmail.com","sentAt":"2021-02-26T10:38:29Z","receivedAt":"2021-02-26T10:40:24Z","isPatch":true,"sender":{"key":"charvi077@gmail.com","avatar":"https://avatars.githubusercontent.com/u/32788121?v=4"},"body":"On Fri, 26 Feb 2021 at 02:30, Junio C Hamano wrote:\n>\n> Charvi Mendiratta writes:\n\n> > Inorder to prevent rebase from creating commits with an empty\n>\n> In order to prevent ...\n>\n> > message we refuse to create an \"amend!\" commit if commit message\n> > body is empty.\n> > ...\n> > +static int prepare_amend_commit(struct commit *commit, struct strbuf *sb,\n> > + struct pretty_print_context *ctx) {\n>\n> Don't indent the second line unnecessarily too deep.\n>\n\nOkay, I will fix the above changes.\n\n> > + /*\n> > + * If we amend the 'amend!' commit then we don't want to\n> > + * duplicate the subject line.\n> > + */\n> > + const char *format = NULL;\n> > + if (starts_with(sb->buf, \"amend! amend!\"))\n> > + format = \"%b\";\n> > + else\n> > + format = \"%B\";\n>\n> I am not sure how well this strategy of special case only two amend!\n> scales. What would happen when we --fixup another \"fixup!\"commit?\n>\n\nThis is applied for more than one \"amend!\" or in other words, \"amend!\"\ncommit chain. On the other hand we don't need to skip any subject if we\n--fixup another fixup! commit because the resulting commit message is\njust one liner. But yes if \"fixup!\" commit is combined with `--squash`\nthen it comments the complete \"fixup!\" commit line by finding its length\nand increasing pointer.\n\n> Shouldn't the caller, when it called format_commit_message() to\n> prepare sb it passed to us, have stripped out existing prefix, if\n> any, so that we can always use the same %B format, or something like\n> that?\n>\n\nI am not sure about this, because I think in the way you have suggested, we need\nto strip off the complete subject line instead of prefix. I am saying\nthis because the\ncommit message body of \"amend!\" commit contains the complete commit message\nof the commit we are fixing up.\nfor example :\n$ git commit --fixup=amend:HEAD will create commit with log message :\namend! subject of head\n\nsubject of head\nbody of head\n\nand again if we `--fixup:amend` the HEAD commit then :\n$ git commit --fixup=amend:HEAD (by default) will create commit with\nlog message:\namend! amend! subject of head\n\namend! subject of head /* we need to comment this complete line */\n\nsubject of head\nbody of head\n\nSo, I am not sure about the other option to implement it ?\n\n> > ...\n> > + format_commit_message(commit, fmt, &sb, &ctx);\n> > + free(fmt);\n> > hook_arg1 = \"message\";\n> > +\n> > + if ((have_option_m) && !strcmp(fixup_prefix,\"fixup\"))\n>\n> Unnecessary () around have_option_m, and missing SP after \",\".\n>\n> > + strbuf_addbuf(&sb, &message);\n> > +\n> > + if (!strcmp(fixup_prefix,\"amend\")) {\n>\n> Missing SP after \",\" (I won't repeat---please re-check the whole\n> patch series before rerolling).\n\nApologies for this. I will take care of it.\n\n>\n> > + if (have_option_m)\n> > + die(_(\"cannot combine -m with --fixup:%s\"), fixup_message);\n> > + else\n> > + prepare_amend_commit(commit, &sb, &ctx);\n>\n> Hmph, why is -m so special? Should we allow --fixup=amend:\n> with -F (or -c/-C for that matter), or are these other options\n> caught at a lot higher layer already and we do not have to check\n> them here?\n>\n\nyes, those options are caught earlier and give the error as below:\n\"Only one of -c/-C/-F/--fixup can be used.\"\nand only `-m` is checked over here.\n\n> > if (also + only + all + interactive > 1)\n> > die(_(\"Only one of --include/--only/--all/--interactive/--patch can be used.\"));\n> > +\n> > + if (fixup_message) {\n> > + /*\n> > + * As `amend` suboption contains only alpha\n> > + * character. So check if first non alpha\n> > + * character in fixup_message is ':'.\n> > + */\n> > + size_t len = get_alpha_len(fixup_message);\n> > + if (len && fixup_message[len] == ':') {\n> > + fixup_message[len] = '\\0';\n> > + fixup_commit = fixup_message + ++len;\n>\n> It would be easier to follow to write it this way, no?\n>\n> fixup_message[len++] = '\\0';\n> fixup_commit = fixup_message + len;\n>\n\nI agree and will update it .\n\nThanks a lot for the reviews. I will do the fixes and update in the\nnext revision.\n\nThanks and Regards,\nCharvi\n"},{"id":"417906","messageId":"xmqqczwmsjzl.fsf@gitster.g","threadId":"55160","inReplyTo":"CAPSFM5eKnqOhDrZWEd-c_sDNMbQkqtMwpFM0zT-gOVHvMtcZnQ@mail.gmail.com","subject":"Re: [PATCH v2 2/6] commit: add amend suboption to --fixup to create amend! commit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-26T19:32:30Z","receivedAt":"2021-02-26T19:33:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"> subject of head\n> body of head\n>\n> So, I am not sure about the other option to implement it ?\n\nThaks, and OK.\n\n>> > + if (have_option_m)\n>> > + die(_(\"cannot combine -m with --fixup:%s\"), fixup_message);\n>> > + else\n>> > + prepare_amend_commit(commit, &sb, &ctx);\n>>\n>> Hmph, why is -m so special? Should we allow --fixup=amend:\n>> with -F (or -c/-C for that matter), or are these other options\n>> caught at a lot higher layer already and we do not have to check\n>> them here?\n>\n> yes, those options are caught earlier and give the error as below:\n> \"Only one of -c/-C/-F/--fixup can be used.\"\n> and only `-m` is checked over here.\n\nAnd the reason why -m cannot be checked early is because we do not\nrecognize which kind of \"fixup\" we are doing when \"only one of\n-c/-C/-F/--fixup\" check is made before this function is called?\n\nOK. I wonder if we can tell which kind of fixup we are doing much\nearlier, though. Then we could extend it to say \"Only one of\n-c/-C/-F/-m/--fixup=amend: can be used\", etc., and we do not\nhave to have this \"only -m is checked here, everything else is\nchecked earlier\" curiosity. But I do not know if such a change is\nnecessarily an improvement. I guess a better \"fix\" would probably\nbe to add a comment to this function where it only checks for \"-m\"\nand tell readers why -c/-C/-F do not have to be checked here.\n\nThanks.\n\n\n"},{"id":"417937","messageId":"CAPSFM5fhaG1fgv30FvOSL-tfpuPiePFYoax_78dvs4Mh8H9tgg@mail.gmail.com","threadId":"55160","inReplyTo":"xmqqczwmsjzl.fsf@gitster.g","subject":"Re: [PATCH v2 2/6] commit: add amend suboption to --fixup to create amend! commit","fromName":"Charvi Mendiratta","fromEmail":"charvi077@gmail.com","sentAt":"2021-02-27T04:56:58Z","receivedAt":"2021-02-27T05:10:31Z","isPatch":true,"sender":{"key":"charvi077@gmail.com","avatar":"https://avatars.githubusercontent.com/u/32788121?v=4"},"body":"On Sat, 27 Feb 2021 at 01:02, Junio C Hamano wrote:\n[...]\n> > yes, those options are caught earlier and give the error as below:\n> > \"Only one of -c/-C/-F/--fixup can be used.\"\n> > and only `-m` is checked over here.\n>\n> And the reason why -m cannot be checked early is because we do not\n> recognize which kind of \"fixup\" we are doing when \"only one of\n> -c/-C/-F/--fixup\" check is made before this function is called?\n>\n> OK. I wonder if we can tell which kind of fixup we are doing much\n> earlier, though. Then we could extend it to say \"Only one of\n> -c/-C/-F/-m/--fixup=amend: can be used\", etc., and we do not\n> have to have this \"only -m is checked here, everything else is\n> checked earlier\" curiosity. But I do not know if such a change is\n> necessarily an improvement. I guess a better \"fix\" would probably\n> be to add a comment to this function where it only checks for \"-m\"\n> and tell readers why -c/-C/-F do not have to be checked here.\n>\n\nI agree, earlier I even asked myself why only `-m`, but this was the\nonly option that was allowing to write/append commit message in `\n--fixup`. And I agree to add a comment, to make it more clear.\n\nThanks and Regards,\nCharvi\n"}]}