| Message ID | 20210217073725.16656-2-charvi077@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | commit: Implementation of "amend!" commit | expand |
Charvi Mendiratta <charvi077@gmail.com> writes: > `git commit --fixup=amend:<commit>` will create an "amend!" commit. > The resulting commit message subject will be "amend! ..." where > "..." is the subject line of <commit> and the initial message > body will be <commit>'s message. -m can be used to override the > message body. > > The "amend!" commit when rebased with --autosquash will fixup the > contents and replace the commit message of <commit> with the > "amend!" commit's message body. > > Inorder to prevent rebase from creating commits with an empty > message we refuse to create an "amend!" commit if commit message > body is empty. > > Example usage: > $ git commit --fixup=amend:HEAD > $ git commit --fixup=amend:HEAD -m "clever commit message" Sorry, but it is not so clear what these examples are trying to illustrate. The first one is to add a new commit to later amend the tip commit (It is a bit of mystery why the user does not do a more usual "git commit --amend" right there, though, and such a mystery may distract readers. If the commit were not at the tip, e.g. HEAD~3, it may be less distracting). The second one, even with s|HEAD|HEAD~3| is even less clear. Without the "-m", the resulting commit will have the subject that begins with !amend but the log message body is taken from the given commit, but with "-m", what happens? Does a single-liner 'clever commit message' _replace_ the log message of the named commit, resulting in an !amend commit that has no message from the original? Or does 'clever commit message' get _appended_ the log message? I think we can just remove the "example" from here and explain the feature well in the end-user facing documentation. > + if (fixup_message) { > + /* > + * check if ':' occurs before '^' or '@', otherwise > + * fixup_message is a commit reference. > + */ Isn't it that you only intend to parse: --fixup --fixup=amend:<any string that names a commit> --fixup=<any string that names a commit> and later extend it to allow keywords other than "amend"? I can understand that you are trying to avoid getting fooled by things like --fixup='HEAD^{/commit message with a colon : in it}' but why special case only ^ and @? This feels brittle (note that I said "things like", exactly because I do not know if any string that can name a commit must have "@" or "^" appear before ":" if it is to have ":" in anywhere, which is what this code assumes). Instead, you can find the first colon, check for known keywords (or a string that consists only of alnums to accomodate for future enhancement), and treat any garbage that happens to have a colon without the "keyword" as fixup_commit. I.e. something along this line... const char alphas[] = "abcde...xyz"; size_t kwd_len; kwd_len = strspn(fixup_message, alphas); if (kwd_len && fixup_message[kwd_len] == ':') { /* found keyword? */ fixup_message[kwd_len] = '\0'; if (!strcmp("amend", fixup_message)) { ... do the amend:<commit> thing ... #if in-next-step-when-you-add-support-for-reword } else if (!strcmp("reword", fixup_message)) { ... do the reword:<commit> thing ... #endif } else { die(_("unknown --fixup=%s:<commit>", fixup_message)); } } else { /* the entire fixup_message is the commit */ }
Hi Junio, On Thu, 18 Feb 2021 at 01:20, Junio C Hamano <gitster@pobox.com> wrote: [...] > The second one, even with s|HEAD|HEAD~3| is even less clear. > Without the "-m", the resulting commit will have the subject that > begins with !amend but the log message body is taken from the given > commit, but with "-m", what happens? Does a single-liner 'clever > commit message' _replace_ the log message of the named commit, > resulting in an !amend commit that has no message from the original? > Or does 'clever commit message' get _appended_ the log message? > Yes, here it gets _appended_ the log message. I agree this seems a bit confusing. > I think we can just remove the "example" from here and explain the > feature well in the end-user facing documentation. > Okay, I will remove it from here and add it in the documentation. > > + if (fixup_message) { > > + /* > > + * check if ':' occurs before '^' or '@', otherwise > > + * fixup_message is a commit reference. > > + */ > > Isn't it that you only intend to parse: > > --fixup > --fixup=amend:<any string that names a commit> > --fixup=<any string that names a commit> > > and later extend it to allow keywords other than "amend"? > Agree. > I can understand that you are trying to avoid getting fooled by > things like > > --fixup='HEAD^{/commit message with a colon : in it}' > > but why special case only ^ and @? This feels brittle (note that I > said "things like", exactly because I do not know if any string that > can name a commit must have "@" or "^" appear before ":" if it is to > have ":" in anywhere, which is what this code assumes). > Okay, I got this... > Instead, you can find the first colon, check for known keywords (or > a string that consists only of alnums to accomodate for future > enhancement), and treat any garbage that happens to have a colon > without the "keyword" as fixup_commit. I.e. something along this > line... > > const char alphas[] = "abcde...xyz"; > size_t kwd_len; > > kwd_len = strspn(fixup_message, alphas); > if (kwd_len && fixup_message[kwd_len] == ':') { > /* found keyword? */ > fixup_message[kwd_len] = '\0'; > if (!strcmp("amend", fixup_message)) { > ... do the amend:<commit> thing ... > #if in-next-step-when-you-add-support-for-reword > } else if (!strcmp("reword", fixup_message)) { > ... do the reword:<commit> thing ... > #endif > } else { > die(_("unknown --fixup=%s:<commit>", > fixup_message)); > } > } else { > /* the entire fixup_message is the commit */ > } > ...Thanks, for pointing this out. Also, in the above method for alnum I think we can initialize an array of alnum[] instead of alphas[]. Or otherwise I was thinking to instead check: if (!isalnum(*c) && *c == ':') i.e to check that first non alnum char in fixup_message is ':' and returning it's position to extract both fixup_prefix and fixup_commit. Will look into it and update in the next revision.
Charvi Mendiratta <charvi077@gmail.com> writes: > Hi Junio, > > On Thu, 18 Feb 2021 at 01:20, Junio C Hamano <gitster@pobox.com> wrote: > [...] >> The second one, even with s|HEAD|HEAD~3| is even less clear. >> Without the "-m", the resulting commit will have the subject that >> begins with !amend but the log message body is taken from the given >> commit, but with "-m", what happens? Does a single-liner 'clever >> commit message' _replace_ the log message of the named commit, >> resulting in an !amend commit that has no message from the original? >> Or does 'clever commit message' get _appended_ the log message? >> > > Yes, here it gets _appended_ the log message. I agree this seems a bit > confusing. In what situation would a user use "-m 'appended pieces of text'" option, together with "--fixup=amend:<commit>"? I am wondering if we want such a "append to" feature, or is it easier to understand for end-users if "-m", "-F", "-C" and "-c" (the common trait of these options is that they contribute to the log message text) are made incompatible with --fixup=amend:<commit>. > ...Thanks, for pointing this out. Also, in the above method for > alnum I think we can initialize an array of alnum[] instead of > alphas[]. Or otherwise I was thinking to instead check: > if (!isalnum(*c) && *c == ':') Sure a loop is fine, or alnum[] is fine, or just alpha[] is OK, I would think. Do you foresee you'd need --fixup=chomp124:<commit>? I somehow doubt it.
Junio C Hamano <gitster@pobox.com> writes: >> ...Thanks, for pointing this out. Also, in the above method for >> alnum I think we can initialize an array of alnum[] instead of >> alphas[]. Or otherwise I was thinking to instead check: >> if (!isalnum(*c) && *c == ':') > > Sure a loop is fine, or alnum[] is fine, or just alpha[] is OK, I > would think. Do you foresee you'd need --fixup=chomp124:<commit>? > I somehow doubt it. Having said that, we may regret if we did not include some punctuation to allow for a multi-word keyword. IOW, "alpha plus dash" might be a reasonable minimum. But what keyword --fixup=<keyword>:<commit> can take is entirely under our control, so it is not all that unreasonable if we just forced our developers some discipline to pick a single-word keyword for any of their future enhancements. It's not like we are opening up extensibility to the end-users, who may complain that the way they can spell their new <keyword> is too limited. So if we already have alpha[] and/or a helper function that does strspn(alpha) that we can reuse elsewhere, I do not think it is worth to try supporting punctuation. Thanks.
Hi Junio, On Fri, 19 Feb 2021 at 00:49, Junio C Hamano <gitster@pobox.com> wrote: [...] > In what situation would a user use "-m 'appended pieces of text'" > option, together with "--fixup=amend:<commit>"? I am wondering if > we want such a "append to" feature, or is it easier to understand > for end-users if "-m", "-F", "-C" and "-c" (the common trait of > these options is that they contribute to the log message text) > are made incompatible with --fixup=amend:<commit>. > For end-users "-m" or "-F" will make it easier to prepare an "amend!" commit. Because using the "-m" reduces the cost of opening an editor to prepare "amend!" commit and it can be done with command line only. So, I think we can keep -m/-F options. (Explained more about "-m" use in next thread) > > ...Thanks, for pointing this out. Also, in the above method for > > alnum I think we can initialize an array of alnum[] instead of > > alphas[]. Or otherwise I was thinking to instead check: > > if (!isalnum(*c) && *c == ':') > > Sure a loop is fine, or alnum[] is fine, or just alpha[] is OK, I > would think. Do you foresee you'd need --fixup=chomp124:<commit>? > I somehow doubt it. For naming the suboptions, I don't see any use of alnum. Earlier, I thought that it could be possible to add the option of _commit name/ID_, although I am not sure about any particular use case of it for future. So I thought of changing it to an alnum[] but I also agree that we can use just alpha[].
On Fri, 19 Feb 2021 at 02:07, Junio C Hamano <gitster@pobox.com> wrote: > > Junio C Hamano <gitster@pobox.com> writes: > > >> ...Thanks, for pointing this out. Also, in the above method for > >> alnum I think we can initialize an array of alnum[] instead of > >> alphas[]. Or otherwise I was thinking to instead check: > >> if (!isalnum(*c) && *c == ':') > > > > Sure a loop is fine, or alnum[] is fine, or just alpha[] is OK, I > > would think. Do you foresee you'd need --fixup=chomp124:<commit>? > > I somehow doubt it. > > Having said that, we may regret if we did not include some > punctuation to allow for a multi-word keyword. IOW, "alpha plus > dash" might be a reasonable minimum. > > But what keyword --fixup=<keyword>:<commit> can take is entirely > under our control, so it is not all that unreasonable if we just > forced our developers some discipline to pick a single-word keyword > for any of their future enhancements. It's not like we are opening > up extensibility to the end-users, who may complain that the way > they can spell their new <keyword> is too limited. So if we already > have alpha[] and/or a helper function that does strspn(alpha) that > we can reuse elsewhere, I do not think it is worth to try supporting > punctuation. > Oh, yes punctuation is another point. Then for now maybe we can just make the helper function to check for alpha[] only. Thanks and Regards, Charvi
Charvi Mendiratta <charvi077@gmail.com> writes: > For end-users "-m" or "-F" will make it easier to prepare an "amend!" > commit. Because using the "-m" reduces the cost of opening an editor > to prepare "amend!" commit and it can be done with command line only. Hmph. That is not very convicing to me. The user is motivated enough to fix a wrong commit log message and replace it with an improved one by using the "--fixup:amend" feature---why would that improved message can sufficiently be written with just an "-m message", which by definition would be much less capable of preparing a well-thought-out message than with an editor? Yes, with "-m", you can add some short string easily at the end of the existing commit message without opening an editor. But I am trying to see why it is a good thing to be able to do so in the first place. It is very easy to make typoes while doing so and it would be hard to fix these typoes, exactly because you are doing so without being in an editor. And the whole point of --fixup=amend is about improving the message (as opposed to --fixup that is to record the contents that have already been improved in the index). This is why I kept asking what the use case would look like. I am trying to find out if you have a good end-user story in mind that we can use in the documentation to sell the feature to end-users, but I am not seeing one. Is the combination of "--fixup=amend" and "-m <msg>" meant to be used primarily "to leave a note to myself" when the user runs --fixup=amend:<commit>, to just record the points to be elaborated when the message is written for real much later? E.g. ... hack hack hack ... ... good stopping point, but cannot afford time to write ... a good log message now $ git commit --fixup=amend:HEAD~2 -m 'talk about X, Y and Z' -a ... hack hack hack ... ... possibly doing something entirely different ... ... finally comes back to finish cleaning up the branch for real ... $ git rebase --autosquash -i origin And one of the insn created in the todo sheet would be amend!, whose commit message has the message taken from the original HEAD~2 PLUS the reminder to the user that s/he needs to talk about X, Y and Z? And the user at that point writes more comprehensive message about these three things? That is a made-up example of what "appending some short strings possibly full of typoes without having to open an editor" could be useful for. I am not sure if it is truly useful, or if it is just a hand wavy excuse not to mark -m/-F incompatible with --fixup=amend without a real merit [*]. Side note: one reason why I do not find it realistic is that it is unlikely that the user would use --fixup=amend to slurp in the original log message when recording "good stopping point, but cannot afford time" fixup. "--squash HEAD~2 -m <msg>" would be much faster to record the "note to myself" reminder, and when the user finally comes back to clean things up, the amount of work to edit the original's message while looking at the "note to myself" appended at the end would not be any different in either workflow. In any case, that was the kind of answer(s) I was looking for when I asked "what is this good for?" question. Thanks.
Hi, On Sat, 20 Feb 2021 at 08:46, Junio C Hamano <gitster@pobox.com> wrote: > > Charvi Mendiratta <charvi077@gmail.com> writes: > > > For end-users "-m" or "-F" will make it easier to prepare an "amend!" > > commit. Because using the "-m" reduces the cost of opening an editor > > to prepare "amend!" commit and it can be done with command line only. > > Hmph. That is not very convicing to me. The user is motivated > enough to fix a wrong commit log message and replace it with an > improved one by using the "--fixup:amend" feature---why would that > improved message can sufficiently be written with just an "-m > message", which by definition would be much less capable of > preparing a well-thought-out message than with an editor? > > Yes, with "-m", you can add some short string easily at the end of > the existing commit message without opening an editor. But I am > trying to see why it is a good thing to be able to do so in the > first place. It is very easy to make typoes while doing so and it > would be hard to fix these typoes, exactly because you are doing so > without being in an editor. And the whole point of --fixup=amend is > about improving the message (as opposed to --fixup that is to record > the contents that have already been improved in the index). > > This is why I kept asking what the use case would look like. I am > trying to find out if you have a good end-user story in mind that we > can use in the documentation to sell the feature to end-users, but I > am not seeing one. > I was in my mind that, as we can easily prepare the commit using `git commit -m "commit message"`. Similarly, we can extend this working with `git commit --fixup=amend:<commit> -m "new commit message"`. And the difference is that in the later one the goal of changing the commit message will be achieved after `git rebase --autosquash` i.e the later command works with sequencer. This will easily allow us to write a new message for the commit we are fixing up through the command line only similar to `git commit -m` and if we need to fixup the commit msg then it can be done without `-m` and the editor gets open with a seeded commit message we want to fixup. Also the same working applies to `reword` option also and may allow to reword the commit msg from command line only. But I am still doubtful, as I agree with above that the main concern is to only fixup the wrong commit message and in that case the "-m" option can't be that much productive or maybe confusing for users. > Is the combination of "--fixup=amend" and "-m <msg>" meant to be > used primarily "to leave a note to myself" when the user runs > --fixup=amend:<commit>, to just record the points to be elaborated > when the message is written for real much later? E.g. > > ... hack hack hack ... > ... good stopping point, but cannot afford time to write > ... a good log message now > $ git commit --fixup=amend:HEAD~2 -m 'talk about X, Y and Z' -a > ... hack hack hack ... > ... possibly doing something entirely different ... > ... finally comes back to finish cleaning up the branch for real ... > $ git rebase --autosquash -i origin > > And one of the insn created in the todo sheet would be amend!, whose > commit message has the message taken from the original HEAD~2 PLUS the > reminder to the user that s/he needs to talk about X, Y and Z? And > the user at that point writes more comprehensive message about these > three things? > But here in this case, after `git rebase --autosquash -i origin`, it will not open editor again and user is not allowed to write more comprehensive message about these three things, unless the user manually changes from automatically converted `pick` to `fixup -C`, to `fixup -c` command in the rebase todo list that gets opened after `git rebase --autosquash`. And for this case `git commit --squash` is more easy to use. > That is a made-up example of what "appending some short strings > possibly full of typoes without having to open an editor" could be > useful for. I am not sure if it is truly useful, or if it is just a > hand wavy excuse not to mark -m/-F incompatible with --fixup=amend > without a real merit [*]. > > Side note: one reason why I do not find it realistic is that it > is unlikely that the user would use --fixup=amend to slurp in > the original log message when recording "good stopping point, > but cannot afford time" fixup. "--squash HEAD~2 -m <msg>" would > be much faster to record the "note to myself" reminder, and when > the user finally comes back to clean things up, the amount of > work to edit the original's message while looking at the "note > to myself" appended at the end would not be any different in > either workflow. > Yes, I also thought the same and agree with it. > In any case, that was the kind of answer(s) I was looking for when I > asked "what is this good for?" question. > Thanks a lot for explaining each perspective in detail. So, now if we use `-m` as an option to write side-note then `--squash` is already available and for fixing the commit message opening the editor is a must expected option. So shall we remove the `-m` option compatibility if `amend`/ `reword` suboptions are passed to `git commit --fixup` ? Thanks and Regards, Charvi
Charvi Mendiratta <charvi077@gmail.com> writes: > Thanks a lot for explaining each perspective in detail. So, now if we > use `-m` as an option to write side-note then `--squash` is already > available and for fixing the commit message opening the editor is a > must expected option. So shall we remove the `-m` option compatibility > if `amend`/ `reword` suboptions are passed to `git commit --fixup` ? FWIW, I do not have a strong opposition against -m/-F that is not rejected when --fixup=amend is in use. It is OK for Git to accept useless combinations of options that do not help end-users. A combination that is not useful will be left unused, which is not a great loss. So, if it is more work to make the code notice when these options are given in useless combinations and stop with an error message than just accepting and doing useless thing, I am OK if we left them as they are. I was just hoping that letting "-m <message>" and "--fixup=amend" used at the same time would support a great use case, and because I didn't think of any, I asked the person who allowed the combination (that is you) what the intended use cases are. Actively supporting a combination because it gives users great value is more satisfying than just leaving useless combination to exist only because it does not actively hurt. When users say "The command can take these two options at the same time, but I cannot figure out what good the combination is for. It feels utterly useless to use them together" it also is much easier to answer them if we can say "because by using these two together, you can do this great thing" instead of having to say "we do not think it makes much sense to use these two options together." Thanks.
On Sun, 21 Feb 2021 at 12:35, Junio C Hamano <gitster@pobox.com> wrote: > > Charvi Mendiratta <charvi077@gmail.com> writes: > > > Thanks a lot for explaining each perspective in detail. So, now if we > > use `-m` as an option to write side-note then `--squash` is already > > available and for fixing the commit message opening the editor is a > > must expected option. So shall we remove the `-m` option compatibility > > if `amend`/ `reword` suboptions are passed to `git commit --fixup` ? > > FWIW, I do not have a strong opposition against -m/-F that is not > rejected when --fixup=amend is in use. It is OK for Git to accept > useless combinations of options that do not help end-users. A > combination that is not useful will be left unused, which is not a > great loss. > So, if it is more work to make the code notice when > these options are given in useless combinations and stop with an > error message Sorry I didn't get this and would like to once confirm here that, are you pointing to output an error message when the `-m`/`-F` option is passed with `git --fixup=amend/reword` ? Because I think we can do this also. Otherwise .... >than just accepting and doing useless thing, I am OK > if we left them as they are. > > I was just hoping that letting "-m <message>" and "--fixup=amend" > used at the same time would support a great use case, and because I > didn't think of any, I asked the person who allowed the combination > (that is you) what the intended use cases are. Actively supporting > a combination because it gives users great value is more satisfying > than just leaving useless combination to exist only because it does > not actively hurt. When users say "The command can take these two > options at the same time, but I cannot figure out what good the > combination is for. It feels utterly useless to use them together" > it also is much easier to answer them if we can say "because by > using these two together, you can do this great thing" instead of > having to say "we do not think it makes much sense to use these two > options together." > ....If we allow both `m` and `F` to work with `git commit --fixup=amend/reword` with the same working as it is doing now i.e to use `m` to write new commit message, upon `--autosquash`, If it is okay? then I also agree to update the documentation more precisely and include the uses when passed with `m` /`F`(not yet added) option. (Please correct me if I am wrong) Thanks and Regards, Charvi
Charvi Mendiratta <charvi077@gmail.com> writes: >> So, if it is more work to make the code notice when >> these options are given in useless combinations and stop with an >> error message > > Sorry I didn't get this and would like to once confirm here that, are > you pointing to output an error message when the `-m`/`-F` option is > passed with `git --fixup=amend/reword` ? Because I think we can do > this also. Otherwise .... If we were to make -m/-F incompatible with these new features, then sure, we'd notice the combination, show an error message and abort. >>than just accepting and doing useless thing, I am OK >> if we left them as they are. > > ....If we allow both `m` and `F` to work with `git commit > --fixup=amend/reword` with the same working as it is doing now i.e to > use `m` to write new commit message, upon `--autosquash`, If it is > okay? then I also agree to update the documentation more precisely and > include the uses when passed with `m` /`F`(not yet added) option. What would that more precise documentation would say, though? "'-m message' gets appended to the message taken from the original commit"? Saying that alone, without explaining why doing such an appending is useful, would puzzle users and makes them ask "but why such a useless feature exist?" and that was why I was trying to figure out what it is useful for with you, which I think we have failed to do so far. My preference at this point is to error out the combination that we cannot figure out how it could be useful at this moment, so that users who find how it would be useful to come to us and present a hopefully good case for using -m <msg> with --fixup=amend:<commit>. I am assuming that allowing the combination at that point is easy, and the user request will give us a good use case we can use in the documentation to explain for what purpose a user may want to use -m <msg> to append a short string at the end. The end users' use case we see at that point might even suggest that it would be more useful to prepend (as opposed to append) the message we get from -m <msg> to the original log message, and such a change will not be possible if we just choose to append without thinking through the use case we intend to support and release "we do not know what good it would do to append with -m <msg>, but that is what the code happens to do now" version to the users, as people will depend on the behaviour of any released versions.
On Mon, 22 Feb 2021 at 23:05, Junio C Hamano <gitster@pobox.com> wrote: [...] > If we were to make -m/-F incompatible with these new features, then > sure, we'd notice the combination, show an error message and abort. > > >>than just accepting and doing useless thing, I am OK > >> if we left them as they are. > > > > ....If we allow both `m` and `F` to work with `git commit > > --fixup=amend/reword` with the same working as it is doing now i.e to > > use `m` to write new commit message, upon `--autosquash`, If it is > > okay? then I also agree to update the documentation more precisely and > > include the uses when passed with `m` /`F`(not yet added) option. > > What would that more precise documentation would say, though? > > "'-m message' gets appended to the message taken from the original > commit"? Saying that alone, without explaining why doing such an > appending is useful, would puzzle users and makes them ask "but why > such a useless feature exist?" and that was why I was trying to > figure out what it is useful for with you, which I think we have > failed to do so far. > > My preference at this point is to error out the combination that we > cannot figure out how it could be useful at this moment, so that > users who find how it would be useful to come to us and present a > hopefully good case for using -m <msg> with --fixup=amend:<commit>. > I am assuming that allowing the combination at that point is easy, > and the user request will give us a good use case we can use in the > documentation to explain for what purpose a user may want to use -m > <msg> to append a short string at the end. The end users' use case > we see at that point might even suggest that it would be more useful > to prepend (as opposed to append) the message we get from -m <msg> > to the original log message, and such a change will not be possible > if we just choose to append without thinking through the use case we > intend to support and release "we do not know what good it would do > to append with -m <msg>, but that is what the code happens to do now" > version to the users, as people will depend on the behaviour of any > released versions. Okay, I admit prepending the msg can be another way. Thanks a lot for clarifying in detail, I completely agree with it and will error out the combination for now. Thanks and Regards, Charvi
diff --git a/builtin/commit.c b/builtin/commit.c index 505fe60956..f2c5ad2e62 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -105,7 +105,8 @@ static const char *template_file; */ static const char *author_message, *author_message_buffer; static char *edit_message, *use_message; -static char *fixup_message, *squash_message; +static char *fixup_message, *fixup_commit, *squash_message; +static const char *fixup_prefix; static int all, also, interactive, patch_interactive, only, amend, signoff; static int edit_flag = -1; /* unspecified */ static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship; @@ -681,6 +682,21 @@ static void adjust_comment_line_char(const struct strbuf *sb) comment_line_char = *p; } +static int prepare_amend_commit(struct commit *commit, struct strbuf *sb, + struct pretty_print_context *ctx) { + /* + * If we amend the 'amend!' commit then we don't want to + * duplicate the subject line. + */ + const char *format = NULL; + if (starts_with(sb->buf, "amend! amend!")) + format = "%b"; + else + format = "%B"; + format_commit_message(commit, format, sb, ctx); + return 0; +} + static int prepare_to_commit(const char *index_file, const char *prefix, struct commit *current_head, struct wt_status *s, @@ -745,15 +761,18 @@ static int prepare_to_commit(const char *index_file, const char *prefix, } else if (fixup_message) { struct pretty_print_context ctx = {0}; struct commit *commit; - commit = lookup_commit_reference_by_name(fixup_message); + char *fmt = xstrfmt("%s! %%s\n\n", fixup_prefix); + commit = lookup_commit_reference_by_name(fixup_commit); if (!commit) - die(_("could not lookup commit %s"), fixup_message); + die(_("could not lookup commit %s"), fixup_commit); ctx.output_encoding = get_commit_output_encoding(); - format_commit_message(commit, "fixup! %s\n\n", - &sb, &ctx); + format_commit_message(commit, fmt, &sb, &ctx); + free(fmt); + hook_arg1 = "message"; if (have_option_m) strbuf_addbuf(&sb, &message); - hook_arg1 = "message"; + else if (!strcmp(fixup_prefix,"amend")) + prepare_amend_commit(commit, &sb, &ctx); } else if (!stat(git_path_merge_msg(the_repository), &statbuf)) { size_t merge_msg_start; @@ -1170,7 +1189,7 @@ static int parse_and_validate_options(int argc, const char *argv[], if (force_author && renew_authorship) die(_("Using both --reset-author and --author does not make sense")); - if (logfile || have_option_m || use_message || fixup_message) + if (logfile || have_option_m || use_message) use_editor = 0; if (0 <= edit_flag) use_editor = edit_flag; @@ -1227,6 +1246,29 @@ static int parse_and_validate_options(int argc, const char *argv[], if (also + only + all + interactive > 1) die(_("Only one of --include/--only/--all/--interactive/--patch can be used.")); + + if (fixup_message) { + /* + * check if ':' occurs before '^' or '@', otherwise + * fixup_message is a commit reference. + */ + fixup_commit = strpbrk(fixup_message, "^@:"); + if (fixup_commit && *fixup_commit == ':' && + fixup_commit != fixup_message) { + *fixup_commit = '\0'; + if (starts_with("amend", fixup_message)) { + fixup_prefix = "amend"; + fixup_commit++; + } else { + die(_("only amend option can be used with --fixup")); + } + } else { + fixup_commit = fixup_message; + fixup_prefix = "fixup"; + use_editor = 0; + } + } + cleanup_mode = get_cleanup_mode(cleanup_arg, use_editor); handle_untracked_files_arg(s); @@ -1504,7 +1546,12 @@ int cmd_commit(int argc, const char **argv, const char *prefix) OPT_CALLBACK('m', "message", &message, N_("message"), N_("commit message"), opt_parse_m), OPT_STRING('c', "reedit-message", &edit_message, N_("commit"), N_("reuse and edit message from specified commit")), OPT_STRING('C', "reuse-message", &use_message, N_("commit"), N_("reuse message from specified commit")), - OPT_STRING(0, "fixup", &fixup_message, N_("commit"), N_("use autosquash formatted message to fixup specified commit")), + /* + * TRANSLATORS: please do not translate [amend:] + * Here "amend" is an option to the --fixup command + * line flag, that creates amend! commit. + */ + OPT_STRING(0, "fixup", &fixup_message, N_("[amend:]commit"), N_("use autosquash formatted message to fixup or amend specified commit")), OPT_STRING(0, "squash", &squash_message, N_("commit"), N_("use autosquash formatted message to squash specified commit")), OPT_BOOL(0, "reset-author", &renew_authorship, N_("the commit is authored by me now (used with -C/-c/--amend)")), OPT_BOOL('s', "signoff", &signoff, N_("add a Signed-off-by trailer")), @@ -1663,6 +1710,19 @@ int cmd_commit(int argc, const char **argv, const char *prefix) exit(1); } + if (fixup_message && starts_with(sb.buf, "amend! ") && + !allow_empty_message) { + struct strbuf body = STRBUF_INIT; + size_t len = subject_length(sb.buf); + strbuf_addstr(&body, sb.buf + len); + if (message_is_empty(&body, cleanup_mode)) { + rollback_index_files(); + fprintf(stderr, _("Aborting commit due to empty commit message body.\n")); + exit(1); + } + strbuf_release(&body); + } + if (amend) { const char *exclude_gpgsig[3] = { "gpgsig", "gpgsig-sha256", NULL }; extra = read_commit_extra_headers(current_head, exclude_gpgsig);
`git commit --fixup=amend:<commit>` will create an "amend!" commit. The resulting commit message subject will be "amend! ..." where "..." is the subject line of <commit> and the initial message body will be <commit>'s message. -m can be used to override the message body. The "amend!" commit when rebased with --autosquash will fixup the contents and replace the commit message of <commit> with the "amend!" commit's message body. Inorder to prevent rebase from creating commits with an empty message we refuse to create an "amend!" commit if commit message body is empty. Example usage: $ git commit --fixup=amend:HEAD $ git commit --fixup=amend:HEAD -m "clever commit message" Mentored-by: Christian Couder <chriscool@tuxfamily.org> Mentored-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Charvi Mendiratta <charvi077@gmail.com> --- builtin/commit.c | 76 +++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 68 insertions(+), 8 deletions(-)