| Message ID | 20210301084512.27170-7-charvi077@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | commit: Implementation of "amend!" commit | expand |
Charvi Mendiratta <charvi077@gmail.com> writes: > +--fixup=[(amend|reword):]<commit>:: > + Without `amend:` or `reword:`, create a `fixup!` commit where > + the commit message will be the subject line from the specified > + commit with a prefix of "fixup!'". The resulting "fixup!" commit What's the single quote in "fixup!'"??? We also have an "amend!'" below. > + is further used with `git rebase --autosquash` to fixup the > + content of the specified commit. > ++ > +The `--fixup=amend:<commit>` form creates an "amend!" commit to > +fixup both the content and the commit log message of the specified > +commit. The resulting "amend!" commit's commit message subject > +will be the subject line from the specified commit with a prefix of > +"amend!'" and the message body will be commit log message of the > +specified commit. It also invokes an editor seeded with the log > ... > +The `--fixup=amend:` and `--fixup=reword:` forms cannot be used with > +other options to add to the commit log message i.e it is incompatible "i.e." > +with `-m`/`-F`/`-c`/`-C` options. > ++
On Mon, Mar 1, 2021 at 3:52 AM Charvi Mendiratta <charvi077@gmail.com> wrote: > Signed-off-by: Charvi Mendiratta <charvi077@gmail.com> > --- > diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt > @@ -9,7 +9,7 @@ SYNOPSIS > + [--dry-run] [(-c | -C | --squash) <commit> | --fixup [(amend|reword):]<commit>)] > @@ -86,11 +86,39 @@ OPTIONS > ---fixup=<commit>:: > +--fixup=[(amend|reword):]<commit>:: Although technically correct, I can't help but wonder if we can be more friendly to readers by rephrasing this as: --fixup=<commit>:: --fixup=amend:<commit>:: --fixup=reword:<commit>:: which is probably a lot easier to take in and understand at a glance. Same comment applies to the synopsis. Not necessarily worth a re-roll. > + Without `amend:` or `reword:`, create a `fixup!` commit where > + the commit message will be the subject line from the specified > + commit with a prefix of "fixup!'". The resulting "fixup!" commit > + is further used with `git rebase --autosquash` to fixup the > + content of the specified commit. I think it becomes important at this point to make it more clear that _only_ the content of <commit> gets changed by the "fixup!" commit, and that the log message of <commit> is untouched. > +The `--fixup=amend:<commit>` form creates an "amend!" commit to > +fixup both the content and the commit log message of the specified > +commit. The resulting "amend!" commit's commit message subject > +will be the subject line from the specified commit with a prefix of > +"amend!'" and the message body will be commit log message of the > +specified commit. It also invokes an editor seeded with the log > +message of the "amend!" commit to allow to edit further. And it > +refuses to create "amend!" commit if it's commit message body is > +empty unless used with the `--allow-empty-message` option. "amend!" > +commit when rebased with `--autosquash` will fixup the contents and > +replace the commit message of the specified commit with the "amend!" > +commit's message body. I had to read this several times to understand what it is trying to say. I believe that part of the problem is that the bulk of the description goes into great detail describing bits and behaviors which make no sense without understanding what an "amend!" commit actually does, which isn't explained until the very last sentence. So, I think the entire description needs to be flipped on its head. In particular, it should start by saying "create a new commit which both fixes up the content of <commit> and replaces <commit>'s log message", and only then dive into the details. In fact, what I just wrote suggests a larger problem with the description of `--fixup` overall. There is no high-level explanation of what a "fixup" (or "amend" or "reword") is; it just dives right into the minutiae without providing the reader with sufficient context to understand any of it. Only a reader who is already familiar with interactive rebase is likely to grok what is being said here. So, extending the thought I expressed above, it would be helpful for the description of `--fixup=[amend:|reword:]` to start by first explaining what a "fixup" is, followed by simple descriptions of "amend" and "reword" (building upon "fixup"), and followed finally by details of each. Very roughly, something like this: Creates a new commit which "fixes up" <commit> when applied with `git rebase --autosquash`. A "fixup" commit changes the content of <commit> but leaves its log message untouched. An "amend" commit is like "fixup" but also replaces the log message of <commit> with the log message of the "amend" commit. A "reword" commit replaces the log message of <commit> with its own log message but makes no changes to the content. And then dive into the details of each variation. > +The `--fixup=amend:` and `--fixup=reword:` forms cannot be used with > +other options to add to the commit log message i.e it is incompatible > +with `-m`/`-F`/`-c`/`-C` options. I suppose it doesn't hurt, but I wonder if it's really necessary to document this considering that the user will learn soon enough upon trying invalid combinations. > +Also, after fixing the commit using `--fixup`, with or without option > +and rebased with `--autosquash`, the authorship of the original commit > +remains unchanged. See linkgit:git-rebase[1] for details. Good. > diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt > @@ -593,16 +593,17 @@ See also INCOMPATIBLE OPTIONS below. > --autosquash:: > --no-autosquash:: > + When the commit log message begins with "squash! ..." (or "fixup! ..." > + or "amend! ..."), and there is already a commit in the todo list that Should this also be mentioning `reword!`? > + matches the same `...`, automatically modify the todo list of > + `rebase -i`, so that the commit marked for squashing comes right after > + the commit to be modified, and change the action of the moved commit > + from `pick` to `squash` (or `fixup` or `fixup -C`) respectively. A commit It's becoming difficult to know which of the "foo!" prefixes get transformed into which sequencer command since there is no longer a one-to-one correspondence between "foo!" prefixes and sequencer commands as there was when only "squash!" and "fixup!" existed. The reader should be told what sequencer command(s) "amend!" and "reword!" become. > + matches the `...` if the commit subject matches, or if the `...` refers > + to the commit's hash. As a fall-back, partial matches of the commit > + subject work, too. The recommended way to create fixup/squash/amend > + commits is by using the `--fixup=[amend|reword]`/`--squash` options of > + linkgit:git-commit[1]. At this point, it may be beneficial to write these out long-form to make it easier on the reader; something along the lines of: ... the `--fixup`, `--fixup:amend:`, `--fixup:reword:`, and `--squash` options of ...
On Tue, 2 Mar 2021 at 00:14, Junio C Hamano <gitster@pobox.com> wrote: > > Charvi Mendiratta <charvi077@gmail.com> writes: > > > +--fixup=[(amend|reword):]<commit>:: > > + Without `amend:` or `reword:`, create a `fixup!` commit where > > + the commit message will be the subject line from the specified > > + commit with a prefix of "fixup!'". The resulting "fixup!" commit > > What's the single quote in "fixup!'"??? We also have an "amend!'" > below. I think I mistook it from the previous discussion and used a single quote instead of space, as earlier it was "fixup " and "amend " which was technically correct. But I will fix it and replace it with "fixup!" and "amend!".
On Tue, 2 Mar 2021 at 12:09, Eric Sunshine <sunshine@sunshineco.com> wrote: > > On Mon, Mar 1, 2021 at 3:52 AM Charvi Mendiratta <charvi077@gmail.com> wrote: > > Signed-off-by: Charvi Mendiratta <charvi077@gmail.com> > > --- > > diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt > > @@ -9,7 +9,7 @@ SYNOPSIS > > + [--dry-run] [(-c | -C | --squash) <commit> | --fixup [(amend|reword):]<commit>)] > > @@ -86,11 +86,39 @@ OPTIONS > > ---fixup=<commit>:: > > +--fixup=[(amend|reword):]<commit>:: > > Although technically correct, I can't help but wonder if we can be > more friendly to readers by rephrasing this as: > > --fixup=<commit>:: > --fixup=amend:<commit>:: > --fixup=reword:<commit>:: > > which is probably a lot easier to take in and understand at a glance. > Same comment applies to the synopsis. > > Not necessarily worth a re-roll. > > > + Without `amend:` or `reword:`, create a `fixup!` commit where > > + the commit message will be the subject line from the specified > > + commit with a prefix of "fixup!'". The resulting "fixup!" commit > > + is further used with `git rebase --autosquash` to fixup the > > + content of the specified commit. > > I think it becomes important at this point to make it more clear that > _only_ the content of <commit> gets changed by the "fixup!" commit, > and that the log message of <commit> is untouched. > > > +The `--fixup=amend:<commit>` form creates an "amend!" commit to > > +fixup both the content and the commit log message of the specified > > +commit. The resulting "amend!" commit's commit message subject > > +will be the subject line from the specified commit with a prefix of > > +"amend!'" and the message body will be commit log message of the > > +specified commit. It also invokes an editor seeded with the log > > +message of the "amend!" commit to allow to edit further. And it > > +refuses to create "amend!" commit if it's commit message body is > > +empty unless used with the `--allow-empty-message` option. "amend!" > > +commit when rebased with `--autosquash` will fixup the contents and > > +replace the commit message of the specified commit with the "amend!" > > +commit's message body. > > I had to read this several times to understand what it is trying to > say. I believe that part of the problem is that the bulk of the > description goes into great detail describing bits and behaviors which > make no sense without understanding what an "amend!" commit actually > does, which isn't explained until the very last sentence. So, I think > the entire description needs to be flipped on its head. In particular, > it should start by saying "create a new commit which both fixes up the > content of <commit> and replaces <commit>'s log message", and only > then dive into the details. > > In fact, what I just wrote suggests a larger problem with the > description of `--fixup` overall. There is no high-level explanation > of what a "fixup" (or "amend" or "reword") is; it just dives right > into the minutiae without providing the reader with sufficient context > to understand any of it. Only a reader who is already familiar with > interactive rebase is likely to grok what is being said here. So, > extending the thought I expressed above, it would be helpful for the > description of `--fixup=[amend:|reword:]` to start by first explaining > what a "fixup" is, followed by simple descriptions of "amend" and > "reword" (building upon "fixup"), and followed finally by details of > each. Very roughly, something like this: > > Creates a new commit which "fixes up" <commit> when applied with > `git rebase --autosquash`. > > A "fixup" commit changes the content of <commit> but leaves its > log message untouched. > > An "amend" commit is like "fixup" but also replaces the log > message of <commit> with the log message of the "amend" commit. > > A "reword" commit replaces the log message of <commit> with its > own log message but makes no changes to the content. > > And then dive into the details of each variation. > Agree, thanks for pointing this out in detail. I will rewrite the doc in the above suggested way and update in the next version. > > +The `--fixup=amend:` and `--fixup=reword:` forms cannot be used with > > +other options to add to the commit log message i.e it is incompatible > > +with `-m`/`-F`/`-c`/`-C` options. > > I suppose it doesn't hurt, but I wonder if it's really necessary to > document this considering that the user will learn soon enough upon > trying invalid combinations. > Not necessary, but I thought that users must know that `-m` is otherwise supported with plain `--fixup` and not with the `amend` and `reword` suboptions. So, I think to reword it and add with the above. > > diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt > > @@ -593,16 +593,17 @@ See also INCOMPATIBLE OPTIONS below. > > --autosquash:: > > --no-autosquash:: > > + When the commit log message begins with "squash! ..." (or "fixup! ..." > > + or "amend! ..."), and there is already a commit in the todo list that > > Should this also be mentioning `reword!`? No, as both `amend` and `reword` suboptions create "amend!" commit only. I think it seems a bit confusing but I will try another attempt to reword the document. > > > + matches the same `...`, automatically modify the todo list of > > + `rebase -i`, so that the commit marked for squashing comes right after > > + the commit to be modified, and change the action of the moved commit > > + from `pick` to `squash` (or `fixup` or `fixup -C`) respectively. A commit > > It's becoming difficult to know which of the "foo!" prefixes get > transformed into which sequencer command since there is no longer a > one-to-one correspondence between "foo!" prefixes and sequencer > commands as there was when only "squash!" and "fixup!" existed. The > reader should be told what sequencer command(s) "amend!" and "reword!" > become. > Okay, I will change it and explain it in more detail. > > + matches the `...` if the commit subject matches, or if the `...` refers > > + to the commit's hash. As a fall-back, partial matches of the commit > > + subject work, too. The recommended way to create fixup/squash/amend > > + commits is by using the `--fixup=[amend|reword]`/`--squash` options of > > + linkgit:git-commit[1]. > > At this point, it may be beneficial to write these out long-form to > make it easier on the reader; something along the lines of: > > ... the `--fixup`, `--fixup:amend:`, `--fixup:reword:`, and > `--squash` options of ... Agree, I will fix it. Thanks for all the detailed reviews and suggestions. I will update all the changes in the next revision. Thanks and Regards, Charvi
On Wed, Mar 3, 2021 at 2:44 AM Charvi Mendiratta <charvi077@gmail.com> wrote: > On Tue, 2 Mar 2021 at 12:09, Eric Sunshine <sunshine@sunshineco.com> wrote: > > On Mon, Mar 1, 2021 at 3:52 AM Charvi Mendiratta <charvi077@gmail.com> wrote: > > > Signed-off-by: Charvi Mendiratta <charvi077@gmail.com> > > > + Without `amend:` or `reword:`, create a `fixup!` commit where > > > + the commit message will be the subject line from the specified > > > + commit with a prefix of "fixup!'". The resulting "fixup!" commit > > > + is further used with `git rebase --autosquash` to fixup the > > > + content of the specified commit. By the way, now that you explained in the other thread that "short" prefix-matching of "amend" and "reword" are allowed, I realize that the documentation doesn't mention it (or at least I don't remember reading it). (Nevertheless, I still feel uncomfortable about supporting short prefix-matching in the initial implementation without any evidence that users will demand it, since we can't change that decision once it's in the hands of users.) > > > + When the commit log message begins with "squash! ..." (or "fixup! ..." > > > + or "amend! ..."), and there is already a commit in the todo list that > > > > Should this also be mentioning `reword!`? > > No, as both `amend` and `reword` suboptions create "amend!" commit > only. I think it seems a bit confusing but I will try another attempt > to reword the document. Hmm, I see. So "reword!" is really just an "amend!" with only commit message but no patch content. That makes perfect sense from an implementation standpoint, but it makes me wonder if it would be easier for users to understand if it created a "reword!" commit which would be recognized as an alias of "amend!". (But maybe that's getting too confusing, and my musing should be ignored.) This also answers an unasked question I had regarding the duplicate "amend! amend!" check. I was wondering why it wasn't also checking for "reword! reword!".
On Wed, 3 Mar 2021 at 13:48, Eric Sunshine <sunshine@sunshineco.com> wrote: > [...] > By the way, now that you explained in the other thread that "short" > prefix-matching of "amend" and "reword" are allowed, I realize that > the documentation doesn't mention it (or at least I don't remember > reading it). > Yes, I admit it was not included. I will add it too. > (Nevertheless, I still feel uncomfortable about supporting short > prefix-matching in the initial implementation without any evidence > that users will demand it, since we can't change that decision once > it's in the hands of users.) > I am not sure about strong evidence but I tried to keep the major points discussed earlier, as mentioned in the previous thread. Also I think otherwise the short prefix will ease out / shorten the command to prepare the "amend!" and also mirrors the commands in interactive rebase. > > > > + When the commit log message begins with "squash! ..." (or "fixup! ..." > > > > + or "amend! ..."), and there is already a commit in the todo list that > > > > > > Should this also be mentioning `reword!`? > > > > No, as both `amend` and `reword` suboptions create "amend!" commit > > only. I think it seems a bit confusing but I will try another attempt > > to reword the document. > > Hmm, I see. So "reword!" is really just an "amend!" with only commit > message but no patch content. That makes perfect sense from an > implementation standpoint, but it makes me wonder if it would be > easier for users to understand if it created a "reword!" commit which > would be recognized as an alias of "amend!". (But maybe that's getting > too confusing, and my musing should be ignored.) > Yes, we didn't choose to make "reword!" commit because if we do so then again it would be expected to implicitly change 'pick' command to 'reword' in sequencer/ rebase to-do list when combined with 'git rebase -- autosquash'. But here we are changing 'pick' to ' fixup -C' to fulfill the working. So, we decided to create a variant of '--fixup' and serve it as "amend!" commit. > This also answers an unasked question I had regarding the duplicate > "amend! amend!" check. I was wondering why it wasn't also checking for > "reword! reword!". Yes, it's true. Thanks for the reveiws, I will add the above mentioned changes too. Thanks and Regards, Charvi
Eric Sunshine <sunshine@sunshineco.com> writes: >> No, as both `amend` and `reword` suboptions create "amend!" commit >> only. I think it seems a bit confusing but I will try another attempt >> to reword the document. > > Hmm, I see. So "reword!" is really just an "amend!" with only commit > message but no patch content. That makes perfect sense from an > implementation standpoint, but it makes me wonder if it would be > easier for users to understand if it created a "reword!" commit which > would be recognized as an alias of "amend!". (But maybe that's getting > too confusing, and my musing should be ignored.) Perhaps related, perhaps not, but I wonder if we really need --fixup=amend and --fixup=reword to begin with. The "amend" variant, $ git commit --fixup=amend:<original> ... other args ... is about shaping the index with "other args" and recording the resulting tree with the log message taken from <original>, marked with the "amend!" prefix. The --fixup=reword:<original> variant is a mere special case of it where the recorded tree is made of the index in the same way as a partial commit with pathspec that matches no paths, i.e. If you have --fixup=amend, you can do $ git commit --fixup=amend:<original> --only and you do not need --fixup=reword:<original> at all, no?
On Thu, 4 Mar 2021 at 06:35, Junio C Hamano <gitster@pobox.com> wrote: > > Eric Sunshine <sunshine@sunshineco.com> writes: > > > Hmm, I see. So "reword!" is really just an "amend!" with only commit > > message but no patch content. That makes perfect sense from an > > implementation standpoint, but it makes me wonder if it would be > > easier for users to understand if it created a "reword!" commit which > > would be recognized as an alias of "amend!". (But maybe that's getting > > too confusing, and my musing should be ignored.) > > Perhaps related, perhaps not, but I wonder if we really need --fixup=amend > and --fixup=reword to begin with. The "amend" variant, > > $ git commit --fixup=amend:<original> ... other args ... > > is about shaping the index with "other args" and recording the > resulting tree with the log message taken from <original>, marked > with the "amend!" prefix. The --fixup=reword:<original> variant > is a mere special case of it where the recorded tree is made of the > index in the same way as a partial commit with pathspec that matches > no paths, i.e. If you have --fixup=amend, you can do > > $ git commit --fixup=amend:<original> --only > > and you do not need --fixup=reword:<original> at all, no? > Maybe as an alternative User interface, we can remove the `--fixup=reword:<original>`. But for this patch, as we have kept separate suboption `--fixup=reword:<original>` , so if now we do `--fixup=amend:<original> --only` then it will return the error as below : fatal: No paths with --include/--only does not make sense. So, `amend` works only if staged changes are present otherwise to change only the commit message `reword` option is there. I agree we can change to the above UI but still I wonder which one is more friendly ? Also, I think we need to add complete `--allow-empty --only` to mirror the working of `reword` so this may result in a lot of typing and hard to remember. Thanks and Regards, Charvi
Charvi Mendiratta <charvi077@gmail.com> writes: >> no paths, i.e. If you have --fixup=amend, you can do >> >> $ git commit --fixup=amend:<original> --only >> >> and you do not need --fixup=reword:<original> at all, no? >> > > Maybe as an alternative User interface, we can remove the > `--fixup=reword:<original>`. > > But for this patch, as we have kept separate suboption > `--fixup=reword:<original>` , so if now we do > `--fixup=amend:<original> --only` then it will return the error as > below : > fatal: No paths with --include/--only does not make sense. Yes, but it is something we can easily fix, just like we made "--only" without any pathname to work with "--amend" (or with "--allow-empty"). The reason I brought it up was not because "--fixup=reword" is not needed as a short-hand for "--only --fixup=amend" (but thinking about it again, I do not think it is so bad), but primarily in response to "would it be easier for users if we had reword! insn in addition to amend! verb in the todo file?" that was raised earlier in the thread. If we position "--fixup=reword" as a short-hand and/or a syntax sugar for "--fixup=amend" and advertise it as such sufficiently to educate users, it would be easier for users to understand why they both result in "amend!".
On Fri, 5 Mar 2021 at 03:48, Junio C Hamano <gitster@pobox.com> wrote: > > Charvi Mendiratta <charvi077@gmail.com> writes: > > >> no paths, i.e. If you have --fixup=amend, you can do > >> > >> $ git commit --fixup=amend:<original> --only > >> > >> and you do not need --fixup=reword:<original> at all, no? > >> > > > > Maybe as an alternative User interface, we can remove the > > `--fixup=reword:<original>`. > > > > But for this patch, as we have kept separate suboption > > `--fixup=reword:<original>` , so if now we do > > `--fixup=amend:<original> --only` then it will return the error as > > below : > > fatal: No paths with --include/--only does not make sense. > > Yes, but it is something we can easily fix, just like we made > "--only" without any pathname to work with "--amend" (or with > "--allow-empty"). > Agree. > The reason I brought it up was not because "--fixup=reword" is not > needed as a short-hand for "--only --fixup=amend" (but thinking > about it again, I do not think it is so bad), but primarily in > response to "would it be easier for users if we had reword! insn in > addition to amend! verb in the todo file?" that was raised earlier > in the thread. If we position "--fixup=reword" as a short-hand > and/or a syntax sugar for "--fixup=amend" and advertise it as such > sufficiently to educate users, it would be easier for users to > understand why they both result in "amend!". Okay, so now if it's Ok to keep the short-hand "--fixup=reword" ? then I think making the documentation more clear would be sufficient to serve it to the users ? Otherwise if the votes are more that the user will get confused as both results in "amend!", then should we change this patch to "--only --fixup=amend" ?
Charvi Mendiratta <charvi077@gmail.com> writes: >> The reason I brought it up was not because "--fixup=reword" is not >> needed as a short-hand for "--only --fixup=amend" (but thinking >> about it again, I do not think it is so bad), but primarily in >> response to "would it be easier for users if we had reword! insn in >> addition to amend! verb in the todo file?" that was raised earlier >> in the thread. If we position "--fixup=reword" as a short-hand >> and/or a syntax sugar for "--fixup=amend" and advertise it as such >> sufficiently to educate users, it would be easier for users to >> understand why they both result in "amend!". > > Okay, so now if it's Ok to keep the short-hand "--fixup=reword" ? then > I think making the documentation more clear would be sufficient to > serve it to the users ? It would be good (1) to keep "--fixup=reword:<commit>" (2) to keep "amend!" but not introduce "reword!" insn (3) document "--fixup=reword:<commit>" can be thought of as a mere special-case short-hand for "--fixup=amend:<commit> --only", and (4) make sure "fixup=amend:<commit> --only" is usable as a replacement for "--fixup=reword:<commit>". but if we are not doing (3) and (4), then it would also be OK to (1) to keep "--fixup=reword:<commit>" (2) to keep "amend!" and introduce "reword!" insn I would think.
On Fri, 5 Mar 2021 at 23:55, Junio C Hamano <gitster@pobox.com> wrote: > > Charvi Mendiratta <charvi077@gmail.com> writes: > > >> The reason I brought it up was not because "--fixup=reword" is not > >> needed as a short-hand for "--only --fixup=amend" (but thinking > >> about it again, I do not think it is so bad), but primarily in > >> response to "would it be easier for users if we had reword! insn in > >> addition to amend! verb in the todo file?" that was raised earlier > >> in the thread. If we position "--fixup=reword" as a short-hand > >> and/or a syntax sugar for "--fixup=amend" and advertise it as such > >> sufficiently to educate users, it would be easier for users to > >> understand why they both result in "amend!". > > > > Okay, so now if it's Ok to keep the short-hand "--fixup=reword" ? then > > I think making the documentation more clear would be sufficient to > > serve it to the users ? > > It would be good > > (1) to keep "--fixup=reword:<commit>" > > (2) to keep "amend!" but not introduce "reword!" insn > > (3) document "--fixup=reword:<commit>" can be thought of as a mere > special-case short-hand for "--fixup=amend:<commit> --only", > and > > (4) make sure "fixup=amend:<commit> --only" is usable as a > replacement for "--fixup=reword:<commit>". > Okay, I agree that this method is more clear ... > but if we are not doing (3) and (4), then it would also be OK to > > (1) to keep "--fixup=reword:<commit>" > > (2) to keep "amend!" and introduce "reword!" insn > ... than this one and will update the patch in the above (former) suggested way. Thanks for suggestions and detailed explanation. Thanks and Regards, Charvi
On Fri, Mar 5, 2021 at 11:13 PM Charvi Mendiratta <charvi077@gmail.com> wrote: > On Fri, 5 Mar 2021 at 23:55, Junio C Hamano <gitster@pobox.com> wrote: > > (1) to keep "--fixup=reword:<commit>" > > > > (2) to keep "amend!" but not introduce "reword!" insn > > > > (3) document "--fixup=reword:<commit>" can be thought of as a mere > > special-case short-hand for "--fixup=amend:<commit> --only", > > and > > > > (4) make sure "fixup=amend:<commit> --only" is usable as a > > replacement for "--fixup=reword:<commit>". > > Okay, I agree that this method is more clear ... This works for me too, especially the bit about improving the documentation to be more clear that --fixup=reword: is a special-case (or syntactic sugar) for --fixup=amend:. My confusion all along was thinking that --fixup=amend: and --fixup=reword: resulted in distinct "amend!" and "reword!" prefixes. I don't know whether that confusion was due to me not reading the commit messages or documentation carefully enough, or because the behavior wasn't clearly documented or easily understood. (I did have to re-read the documentation patch multiple times in an attempt to understand what it was saying, so perhaps I can blame that. ;-) At any rate, it will be good if we can get it clearly documented. > > but if we are not doing (3) and (4), then it would also be OK to > > > > (1) to keep "--fixup=reword:<commit>" > > > > (2) to keep "amend!" and introduce "reword!" insn > > ... than this one and will update the patch in the above (former) suggested way. This option would likely be less desirable since it could confuse people into thinking that "reword!" would become "reword" in the sequencer instruction sheet -- which isn't the case at all -- it becomes "fixup -c" (or -C, I can't remember).
diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt index 17150fa7ea..722f8f0a6d 100644 --- a/Documentation/git-commit.txt +++ b/Documentation/git-commit.txt @@ -9,7 +9,7 @@ SYNOPSIS -------- [verse] 'git commit' [-a | --interactive | --patch] [-s] [-v] [-u<mode>] [--amend] - [--dry-run] [(-c | -C | --fixup | --squash) <commit>] + [--dry-run] [(-c | -C | --squash) <commit> | --fixup [(amend|reword):]<commit>)] [-F <file> | -m <msg>] [--reset-author] [--allow-empty] [--allow-empty-message] [--no-verify] [-e] [--author=<author>] [--date=<date>] [--cleanup=<mode>] [--[no-]status] @@ -86,11 +86,39 @@ OPTIONS Like '-C', but with `-c` the editor is invoked, so that the user can further edit the commit message. ---fixup=<commit>:: - Construct a commit message for use with `rebase --autosquash`. - The commit message will be the subject line from the specified - commit with a prefix of "fixup! ". See linkgit:git-rebase[1] - for details. +--fixup=[(amend|reword):]<commit>:: + Without `amend:` or `reword:`, create a `fixup!` commit where + the commit message will be the subject line from the specified + commit with a prefix of "fixup!'". The resulting "fixup!" commit + is further used with `git rebase --autosquash` to fixup the + content of the specified commit. ++ +The `--fixup=amend:<commit>` form creates an "amend!" commit to +fixup both the content and the commit log message of the specified +commit. The resulting "amend!" commit's commit message subject +will be the subject line from the specified commit with a prefix of +"amend!'" and the message body will be commit log message of the +specified commit. It also invokes an editor seeded with the log +message of the "amend!" commit to allow to edit further. And it +refuses to create "amend!" commit if it's commit message body is +empty unless used with the `--allow-empty-message` option. "amend!" +commit when rebased with `--autosquash` will fixup the contents and +replace the commit message of the specified commit with the "amend!" +commit's message body. ++ +The `--fixup=reword:<commit>` form creates an "amend!" commit similar +to `--fixup=amend:<commit>` creates, but it records the same tree as +`HEAD`, i.e. it does not take any staged changes and only allows to +fixup the commit message of the specified commit. It will reword the +specified commit when it is rebased with `--autosquash`. ++ +The `--fixup=amend:` and `--fixup=reword:` forms cannot be used with +other options to add to the commit log message i.e it is incompatible +with `-m`/`-F`/`-c`/`-C` options. ++ +Also, after fixing the commit using `--fixup`, with or without option +and rebased with `--autosquash`, the authorship of the original commit +remains unchanged. See linkgit:git-rebase[1] for details. --squash=<commit>:: Construct a commit message for use with `rebase --autosquash`. diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index 8bfa5a9272..ffea76e53b 100644 --- a/Documentation/git-rebase.txt +++ b/Documentation/git-rebase.txt @@ -593,16 +593,17 @@ See also INCOMPATIBLE OPTIONS below. --autosquash:: --no-autosquash:: - When the commit log message begins with "squash! ..." (or - "fixup! ..."), and there is already a commit in the todo list that - matches the same `...`, automatically modify the todo list of rebase - -i so that the commit marked for squashing comes right after the - commit to be modified, and change the action of the moved commit - from `pick` to `squash` (or `fixup`). A commit matches the `...` if - the commit subject matches, or if the `...` refers to the commit's - hash. As a fall-back, partial matches of the commit subject work, - too. The recommended way to create fixup/squash commits is by using - the `--fixup`/`--squash` options of linkgit:git-commit[1]. + When the commit log message begins with "squash! ..." (or "fixup! ..." + or "amend! ..."), and there is already a commit in the todo list that + matches the same `...`, automatically modify the todo list of + `rebase -i`, so that the commit marked for squashing comes right after + the commit to be modified, and change the action of the moved commit + from `pick` to `squash` (or `fixup` or `fixup -C`) respectively. A commit + matches the `...` if the commit subject matches, or if the `...` refers + to the commit's hash. As a fall-back, partial matches of the commit + subject work, too. The recommended way to create fixup/squash/amend + commits is by using the `--fixup=[amend|reword]`/`--squash` options of + linkgit:git-commit[1]. + If the `--autosquash` option is enabled by default using the configuration variable `rebase.autoSquash`, this option can be
Mentored-by: Christian Couder <chriscool@tuxfamily.org> Mentored-by: Phillip Wood <phillip.wood@dunelm.org.uk> Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Charvi Mendiratta <charvi077@gmail.com> --- Documentation/git-commit.txt | 40 ++++++++++++++++++++++++++++++------ Documentation/git-rebase.txt | 21 ++++++++++--------- 2 files changed, 45 insertions(+), 16 deletions(-)