Keep original author with git merge --squash?

Keep original author with git merge --squash?

[David Glasser]

I frequently find myself using `git merge --squash` to combine a
series of commits by the same author into one.

(For example, I fetch my project's GitHub PRs into my repo.
Frequently, a PR consists of the original PR (with a good description)
followed by a few follow-ups based on feedback from me.  While I'd
prefer that the submitter amended their original commit instead of
making it my job, this is rare.  And I don't feel that it's valuable
to my project's git history to contain all the intermediate stages of
code review --- it's usually just one commit.)

So `git merge --squash origin/pr/1234` is a really convenient command
here... except for one thing: it sets the author as me.  I always have
to manually find the author line and make sure to pass it to --author
(perhaps with --amend).

What would people think of a flag (or a config value) that means "if
all merged commits are by the same author, use that author for the
resulting commit instead of the default author"?

(I'm not sure if this should be a flag to --squash or to commit.
Maybe `git merge --squash`; `git commit --use-squashed-author`?  Seems
like it should be not too hard to implement; SQUASH_MSG is pretty
parseable.  Or just a config value.)

--dave

-- 
glasser@davidglasser.net | langtonlabs.org | flickr.com/photos/glasser/

Re: Keep original author with git merge --squash?

[Jeff King]

On Wed, Feb 11, 2015 at 09:21:04AM -0800, David Glasser wrote:

> (I'm not sure if this should be a flag to --squash or to commit.
> Maybe `git merge --squash`; `git commit --use-squashed-author`?  Seems
> like it should be not too hard to implement; SQUASH_MSG is pretty
> parseable.  Or just a config value.)

It sounds like "git commit -c" is close to what you want, which will
pull the author and commit message from a particular commit. But I don't
think there is a convenient way to name the commit in your case (it is
likely to be the first commit on the branch you are squash-merging, but
there isn't a shorthand for that).

I assume you are already munging in your editor the template provided by
"git commit" after the squash? What would be really nice, IMHO, is if
there was a way to set the author during that edit (e.g., by moving one
of the "Author:" lines to the top of the file). That would cover your
use case, I think, and would also be useful in general.

-Peff

Re: Keep original author with git merge --squash?

[Michael Haggerty]

On 02/12/2015 10:28 AM, Jeff King wrote:
> On Wed, Feb 11, 2015 at 09:21:04AM -0800, David Glasser wrote:
> 
>> (I'm not sure if this should be a flag to --squash or to commit.
>> Maybe `git merge --squash`; `git commit --use-squashed-author`?  Seems
>> like it should be not too hard to implement; SQUASH_MSG is pretty
>> parseable.  Or just a config value.)
> 
> It sounds like "git commit -c" is close to what you want, which will
> pull the author and commit message from a particular commit. But I don't
> think there is a convenient way to name the commit in your case (it is
> likely to be the first commit on the branch you are squash-merging, but
> there isn't a shorthand for that).
> 
> I assume you are already munging in your editor the template provided by
> "git commit" after the squash? What would be really nice, IMHO, is if
> there was a way to set the author during that edit (e.g., by moving one
> of the "Author:" lines to the top of the file). That would cover your
> use case, I think, and would also be useful in general.

If only Git supported options on todo list lines [1], this could be
implemented via a "--use-author" option:

    pick --use-author 1234556 blah
    squash 84392ab etc
    fixup 49106a5 another

Happily, this would work with fixup, too, without forcing the user to go
into the editor. Also, it wouldn't require metadata to be read in-band
from the commit message.

Michael

[1] http://article.gmane.org/gmane.comp.version-control.git/255410

-- 
Michael Haggerty
mhagger@alum.mit.edu

Re: Keep original author with git merge --squash?

[Jeff King]

On Thu, Feb 12, 2015 at 12:35:48PM +0100, Michael Haggerty wrote:

> > I assume you are already munging in your editor the template provided by
> > "git commit" after the squash? What would be really nice, IMHO, is if
> > there was a way to set the author during that edit (e.g., by moving one
> > of the "Author:" lines to the top of the file). That would cover your
> > use case, I think, and would also be useful in general.
> 
> If only Git supported options on todo list lines [1], this could be
> implemented via a "--use-author" option:
> 
>     pick --use-author 1234556 blah
>     squash 84392ab etc
>     fixup 49106a5 another
> 
> Happily, this would work with fixup, too, without forcing the user to go
> into the editor. Also, it wouldn't require metadata to be read in-band
> from the commit message.

Yes, that would be nice, but I don't think David is using a sequencer
todo list here at all. It's just:

  git merge --squash pr/100
  git commit

-Peff

Re: Keep original author with git merge --squash?

[David Glasser]

On Thu, Feb 12, 2015 at 4:12 AM, Jeff King <peff@peff.net> wrote:
> On Thu, Feb 12, 2015 at 12:35:48PM +0100, Michael Haggerty wrote:
>
>> > I assume you are already munging in your editor the template provided by
>> > "git commit" after the squash? What would be really nice, IMHO, is if
>> > there was a way to set the author during that edit (e.g., by moving one
>> > of the "Author:" lines to the top of the file). That would cover your
>> > use case, I think, and would also be useful in general.
>>
>> If only Git supported options on todo list lines [1], this could be
>> implemented via a "--use-author" option:
>>
>>     pick --use-author 1234556 blah
>>     squash 84392ab etc
>>     fixup 49106a5 another
>>
>> Happily, this would work with fixup, too, without forcing the user to go
>> into the editor. Also, it wouldn't require metadata to be read in-band
>> from the commit message.
>
> Yes, that would be nice, but I don't think David is using a sequencer
> todo list here at all. It's just:
>
>   git merge --squash pr/100
>   git commit

That's correct. I certainly know how to do

  git checkout pr/100
  git rebase -i master  # set things to squash, etc
  git checkout master
  git merge pr/100  # or cherry-pick or whatever

And that's how I always used to do it.  But `merge --squash` is so
much more convenient... except for the minor wrinkle of needing an
extra manual step to give the original author their credit.

--dave


-- 
glasser@davidglasser.net | langtonlabs.org | flickr.com/photos/glasser/

Re: Keep original author with git merge --squash?

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Wed, Feb 11, 2015 at 09:21:04AM -0800, David Glasser wrote:
>
>> (I'm not sure if this should be a flag to --squash or to commit.
>> Maybe `git merge --squash`; `git commit --use-squashed-author`?  Seems
>> like it should be not too hard to implement; SQUASH_MSG is pretty
>> parseable.  Or just a config value.)
>
> It sounds like "git commit -c" is close to what you want, which will
> pull the author and commit message from a particular commit. But I don't
> think there is a convenient way to name the commit in your case (it is
> likely to be the first commit on the branch you are squash-merging, but
> there isn't a shorthand for that).

I thought David was primarily interested in the case where a branch
authored by a single person, so specifying the tip of the branch
being "merged" would be sufficient, no?

Re: Keep original author with git merge --squash?

[David Glasser]

On Thu, Feb 12, 2015 at 12:18 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Jeff King <peff@peff.net> writes:
>
>> On Wed, Feb 11, 2015 at 09:21:04AM -0800, David Glasser wrote:
>>
>>> (I'm not sure if this should be a flag to --squash or to commit.
>>> Maybe `git merge --squash`; `git commit --use-squashed-author`?  Seems
>>> like it should be not too hard to implement; SQUASH_MSG is pretty
>>> parseable.  Or just a config value.)
>>
>> It sounds like "git commit -c" is close to what you want, which will
>> pull the author and commit message from a particular commit. But I don't
>> think there is a convenient way to name the commit in your case (it is
>> likely to be the first commit on the branch you are squash-merging, but
>> there isn't a shorthand for that).
>
> I thought David was primarily interested in the case where a branch
> authored by a single person, so specifying the tip of the branch
> being "merged" would be sufficient, no?

Well, using -c appears to override SQUASH_MSG entirely; it replaces
the message as well as the author.  Often I do want to make my own
message based on all the messages provided by the submitter.  (And
typically the branch's tip is the least useful message anyway: it's
usually something like "respond to code review".)

--dave


-- 
glasser@davidglasser.net | langtonlabs.org | flickr.com/photos/glasser/

Re: Keep original author with git merge --squash?

[Junio C Hamano]

David Glasser <glasser@davidglasser.net> writes:

> Well, using -c appears to override SQUASH_MSG entirely; it replaces
> the message as well as the author.  Often I do want to make my own
> message based on all the messages provided by the submitter.  (And
> typically the branch's tip is the least useful message anyway: it's
> usually something like "respond to code review".)

I wonder if there is a bug in the workflow.  Isn't the contributor
forcing more work to the integrator that way and making it a
responsibility of the integrator to squash multiple commits into
one, instead of asking a cleaned-up branch to be merged in the first
place?  It is a key to keep your project scalable to push as much
work out of the integrator's plate to the contributors' plates.

But that is an unrelated tangent.  Among the ideas floated in the
thread, I tend to like something like what Peff mentioned earlier.

> I assume you are already munging in your editor the template provided by
> "git commit" after the squash? What would be really nice, IMHO, is if
> there was a way to set the author during that edit (e.g., by moving one
> of the "Author:" lines to the top of the file). That would cover your
> use case, I think, and would also be useful in general.

I don't know if the exact solution Peff mentioned is good [*1*], but
being able to do an equivalent of setting --author="<author>" and
"--date=<date>" from the command line inside the log message editor
would be a huge win.


[Footnote]

*1* You would see

	gostak: do not distim doshes unconditionally

	Usually it is a good idea to let gostak distim doshes,
        but it should cause the program segfault when there is
        no doshes to be distimmed.  Detect this case and error
        out.

	Signed-off-by: David Glasser <glasser@davidglasser.net>

        # Please enter the commit message ...
        #
        # Author:    David Glasser <glasser@davidglasser.net>
        # Date:      Thu, 12 Feb 2015 04:28:24 -0500
        # ...

when you start "git commit --amend" or "git merge --squash", and the
proposal, as I understand it, is to allow you to do this:

        From: David Glasser <glasser@davidglasser.net>
        Date: Thu, 12 Feb 2015 04:28:24 -0500

	gostak: do not distim doshes unconditionally

	Usually it is a good idea to let gostak distim doshes,
        but it should cause the program segfault when there is
        no doshes to be distimmed.  Detect this case and error
        out.

	Signed-off-by: David Glasser <glasser@davidglasser.net>

        # Please enter the commit message ...
        #
        # ...

which is the same format of the _body_ of the message "git am" would
take.  The reason I am not so sure about such a change is that
having to move and edit things would be error prone.  Some may
forget to do s/Author:/From:/ and we would end up having to support
both.  Some existing commit may validly have lines that begin with
these tokens ("git am" does not have such a problem because it is
not the underlying "git commit" or "git commit-tree" that interprets
these in-body headers).  Some users may forget to leave a blank line
between the in-body headers and the subject line.

I suspect that it _might_ work better if we always look for the pair
of "# Author: " and "# Date:" lines (taking the commentchar setting
in to account, of course) immediately at the beginning of the
trailing comment block, and if exists, use that to override the
authorship identity.  If the user does not do anything when running
"git commit --amend", the resulting commit will get the original
authorship.  If the user strips all the lines in the trailing
comment block, we would do the same thing as we have always done and
retain the original authorship.

I dunno.

Re: Keep original author with git merge --squash?

[David Glasser]

On Thu, Feb 12, 2015 at 1:23 PM, Junio C Hamano <gitster@pobox.com> wrote:
> David Glasser <glasser@davidglasser.net> writes:
>
>> Well, using -c appears to override SQUASH_MSG entirely; it replaces
>> the message as well as the author.  Often I do want to make my own
>> message based on all the messages provided by the submitter.  (And
>> typically the branch's tip is the least useful message anyway: it's
>> usually something like "respond to code review".)
>
> I wonder if there is a bug in the workflow.  Isn't the contributor
> forcing more work to the integrator that way and making it a
> responsibility of the integrator to squash multiple commits into
> one, instead of asking a cleaned-up branch to be merged in the first
> place?  It is a key to keep your project scalable to push as much
> work out of the integrator's plate to the contributors' plates.
>
> But that is an unrelated tangent.  Among the ideas floated in the
> thread, I tend to like something like what Peff mentioned earlier.

Yes, I agree that the GitHub pull request workflow has some major
disadvantages. It does a good job of enabling unsophisticated
contributors who aren't git experts to propose changes to a codebase,
but not a very good job of encouraging them to do the work of
massaging what may be a series of changes as the patch undergoes code
review into a single patch (or well-structured series of patches).  At
this point in time, the tradeoff of requiring a little more integrator
work makes sense for my project.  I'd rather spend my limited "educate
potential contributors" energy on ensuring that they include actual
reproductions with their bug reports than on getting them to do
administrative git work that isn't hard for me.

So to be concrete: What I'm proposing (and I'm excited to implement
it!) is the following:

When running "git commit" and:
- You've fallen into the case where the message was read from SQUASH_MSG
- You haven't used another method of specifying the author (--author,
  -C, -c, --amend)
- You have not specified --reset-author
- You have set the "commit.useSquashAuthor" option
- Before invoking prepare-commit-msg, all of the `Author:` lines found
  in SQUASH_MSG have the same value

then that author is used, as if it were specified with --author.  (And
this will show up, commented-out, at the bottom of COMMIT_EDITMSG.)

If you have the config option set but you don't want this logic to
take place for a particular post-squash merge, just specify
--reset-author.

I think this makes git more internally consistent, since it makes
`merge --squash` act more like the squash rebase action.  (Personally
I'd be happy if this behavior was the default, but since it is not
backwards-compatible I've included a config option in my
proposal.)

(It is my understanding, based on reading the code, that the format of
SQUASH_MSG is not user-configurable, and that scanning for Author:
lines in it is straightforward.)

--dave

-- 
glasser@davidglasser.net | langtonlabs.org | flickr.com/photos/glasser/

Re: Keep original author with git merge --squash?

[David Glasser]

On Thu, Feb 12, 2015 at 2:16 PM, David Glasser <glasser@davidglasser.net> wrote:
> - Before invoking prepare-commit-msg, all of the `Author:` lines found
>   in SQUASH_MSG have the same value

OK, and to be very specific: I'm just proposing "literally the same
text written after Author"; using mailmap to detect that multiple
different Author lines are actually the same author could be a
potential later improvement.

-- 
glasser@davidglasser.net | langtonlabs.org | flickr.com/photos/glasser/

Re: Keep original author with git merge --squash?

[Junio C Hamano]

David Glasser <glasser@davidglasser.net> writes:

> So to be concrete: What I'm proposing (and I'm excited to implement
> it!) is the following:
>
> When running "git commit" and:
> - You've fallen into the case where the message was read from SQUASH_MSG
> - You haven't used another method of specifying the author (--author,
>   -C, -c, --amend)
> - You have not specified --reset-author
> - You have set the "commit.useSquashAuthor" option
> - Before invoking prepare-commit-msg, all of the `Author:` lines found
>   in SQUASH_MSG have the same value
>
> then that author is used, as if it were specified with --author.  (And
> this will show up, commented-out, at the bottom of COMMIT_EDITMSG.)


I actually was hoping that this would extend to cases other than
"git merge --squash".

When running "git commit" and:

 - You didn't use a more explicit method of specifying the
   authorship identity (--author, --date, -C, -c --amend,
   --reset-author options, or environment variables GIT_AUTHOR_*);

 - You have commit.useAuthorFromEditorComment variable;

 - You have "# Author: " line that are identical in the result of
   the editor,

then use that author.  That would allow "git commit --amend" to
update a misspelled author name, for example.

Is that a bit too liberal?  Would it invite mistakes?

Re: Keep original author with git merge --squash?

[Jeff King]

On Thu, Feb 12, 2015 at 02:34:43PM -0800, Junio C Hamano wrote:

> I actually was hoping that this would extend to cases other than
> "git merge --squash".
> 
> When running "git commit" and:
> 
>  - You didn't use a more explicit method of specifying the
>    authorship identity (--author, --date, -C, -c --amend,
>    --reset-author options, or environment variables GIT_AUTHOR_*);
> 
>  - You have commit.useAuthorFromEditorComment variable;
> 
>  - You have "# Author: " line that are identical in the result of
>    the editor,
> 
> then use that author.  That would allow "git commit --amend" to
> update a misspelled author name, for example.
> 
> Is that a bit too liberal?  Would it invite mistakes?

I like this direction in general.

What happens if there is no "Author:" line in the output? Do we do the
equivalent "--reset-author"? That seems slightly error-prone to me. It
is not uncommon for me to delete to the end file in my editor to drop
cruft (e.g., in an interactive rebase with a "squash" command, I very
often drop the final commit message, and it is simpler to just delete to
the end of file than to delete to the top of the comment block).

I wonder if we should have some markers in the commented-out section
that indicate it even exists, like:

  # --- Lines in this section affect the commit authorship ---
  # Author: ...
  # ---

Of course that is nice when you are editing an existing Author line, but
not so much when you have to remember to type that line yourself
(because you are adding an author attribution when there was not one
before).

So probably a saner thing is that a missing "Author:" line does nothing,
and using "Author: " (with no text) does a reset.

Also, on the topic of "merge --squash". I never use it myself, but
having experimented with it due to this thread, I found the template it
sticks into COMMIT_EDITMSG to be horribly unfriendly for munging. For
example, with two simple commits, I get:

    Squashed commit of the following:
    
    commit 6821a8ac920ed00675e4aec10dcef705211105cd
    Author: Jeff King <peff@peff.net>
    Date:   Thu Feb 12 17:39:28 2015 -0500
    
        commit subject 2
    
        commit body 2
    
    commit b0840bb4bbfe00b6ed8c7c4d483f11d126fa2d69
    Author: Jeff King <peff@peff.net>
    Date:   Thu Feb 12 17:39:28 2015 -0500
    
        commit subject 1
    
        commit body 1

I guess that is helpful if you want to keep a complete log of what got
squashed, but I doubt that is the common case (if you did, then doing a
real merge would probably be in order). But to munge that into a usable
single commit message, I have to:

  1. Drop the header fields. We could mark these with "#" instead (which
     would also make the "# Author: " proposal here work.

  2. Reindent all of the actual message lines!

  3. Probably reorder the commit messages, since they are
     reverse-chronological here.

I would find something like:

    # commit b0840bb4bbfe00b6ed8c7c4d483f11d126fa2d69
    # Author: Jeff King <peff@peff.net>
    # Date:   Thu Feb 12 17:39:28 2015 -0500
    
    commit subject 1
    
    commit body 1

    # ... and then the second commit ...

much more friendly, and closer to what interactive rebase's squash does.
It also raises a question for the proposal in this thread: if there are
multiple "Author:" lines, which one do we take? The first, or the last?
I think in the proposed chronological-order format I just showed, it
would make sense to take the first.

-Peff

Re: Keep original author with git merge --squash?

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> What happens if there is no "Author:" line in the output?

I've been assuming that we would do what the current code does.
"git commit --amend" for example internally remembers who the
original author was and uses that, without paying any attention to
the result from the editor.  If there is no "Author:", that would
not change.

And I do not think we need to be able to say "Oops, I forgot to pass
the --reset-author option from the command line", personally, so...

> So probably a saner thing is that a missing "Author:" line does nothing,

yes and

> and using "Author: " (with no text) does a reset.

no (I do not think it is wrong per-se, but I do not think such a
good idea).

> Also, on the topic of "merge --squash". I never use it myself, but
> having experimented with it due to this thread, I found the template it
> sticks into COMMIT_EDITMSG to be horribly unfriendly for munging. For
> example, with two simple commits, I get:
>
>     Squashed commit of the following:
>     
>     commit 6821a8ac920ed00675e4aec10dcef705211105cd
>     Author: Jeff King <peff@peff.net>
>     Date:   Thu Feb 12 17:39:28 2015 -0500
>     
>         commit subject 2
>     
>         commit body 2
>     
>     commit b0840bb4bbfe00b6ed8c7c4d483f11d126fa2d69
>     Author: Jeff King <peff@peff.net>
>     Date:   Thu Feb 12 17:39:28 2015 -0500
>     
>         commit subject 1
>     
>         commit body 1
>
> I guess that is helpful if you want to keep a complete log of what got
> squashed, but I doubt that is the common case (if you did, then doing a
> real merge would probably be in order).

I think it should show exactly the same thing as "rebase -i" squash
insn would give you.  People already know how to munge that, right?

> It also raises a question for the proposal in this thread: if there are
> multiple "Author:" lines, which one do we take? The first, or the last?

I was siding with David's "pay attention to in-buffer Author: only
when all of them agree".  When squash-merging a branch with two or
more authors, we would attribute the authorship silently and
automatically to you if you do not do anything special otherwise.

Possible alternatives when multiple "Author:"s do not agree are:

 - use you who is playing the integrator;

 - use the tip;

 - use the one that most often appears; or

 - error out and ask the user to leave only one (or zero--if you
   want to take the authorship) by re-attempting "git commit".

Re: Keep original author with git merge --squash?

[Jeff King]

On Thu, Feb 12, 2015 at 03:32:37PM -0800, Junio C Hamano wrote:

> > and using "Author: " (with no text) does a reset.
> 
> no (I do not think it is wrong per-se, but I do not think such a
> good idea).

Fair enough. It is probably a minority use case, and one that is likely
to cause confusion.

> > Also, on the topic of "merge --squash". I never use it myself, but
> > having experimented with it due to this thread, I found the template it
> > sticks into COMMIT_EDITMSG to be horribly unfriendly for munging. For
> > example, with two simple commits, I get:
> [...]
> I think it should show exactly the same thing as "rebase -i" squash
> insn would give you.  People already know how to munge that, right?

Yeah, that was exactly what I expected to see (but didn't). They should
probably have the same format, though we may want to enhance both to
contain more information (like author names).

> > It also raises a question for the proposal in this thread: if there are
> > multiple "Author:" lines, which one do we take? The first, or the last?
> 
> I was siding with David's "pay attention to in-buffer Author: only
> when all of them agree".  When squash-merging a branch with two or
> more authors, we would attribute the authorship silently and
> automatically to you if you do not do anything special otherwise.

That's probably reasonable. I was thinking more of a case where you made
some fixups on top of somebody else's branch, and then used "git rebase
-i" to squash them together. But I think we already use the authorship
for the root of the squash in that case.

This case collapses nicely if we make a slight tweak to your proposed
behavior (or maybe this is what you meant). If there are multiple
authors listed, we behave as if none was listed. That would leave the
authorship as it behaves today (with the author of the first commit) if
you do nothing, or you can override it by dropping all but one.

-Peff

Re: Keep original author with git merge --squash?

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Thu, Feb 12, 2015 at 03:32:37PM -0800, Junio C Hamano wrote:
>
>> > It also raises a question for the proposal in this thread: if there are
>> > multiple "Author:" lines, which one do we take? The first, or the last?
>> 
>> I was siding with David's "pay attention to in-buffer Author: only
>> when all of them agree".  When squash-merging a branch with two or
>> more authors, we would attribute the authorship silently and
>> automatically to you if you do not do anything special otherwise.
>
> That's probably reasonable. I was thinking more of a case where you made
> some fixups on top of somebody else's branch, and then used "git rebase
> -i" to squash them together. But I think we already use the authorship
> for the root of the squash in that case.
>
> This case collapses nicely if we make a slight tweak to your proposed
> behavior (or maybe this is what you meant). If there are multiple
> authors listed, we behave as if none was listed. That would leave the
> authorship as it behaves today (with the author of the first commit) if
> you do nothing, or you can override it by dropping all but one.

I actually was (and am still) wondering that "silently ignore all of
them if there are multiple ones that contradict with each other" is
a bad idea, and that was why the last item on the "possible
alternatives" list was to error out and ask clarification.

Re: Keep original author with git merge --squash?

[Jeff King]

On Fri, Feb 13, 2015 at 11:30:53AM -0800, Junio C Hamano wrote:

> > This case collapses nicely if we make a slight tweak to your proposed
> > behavior (or maybe this is what you meant). If there are multiple
> > authors listed, we behave as if none was listed. That would leave the
> > authorship as it behaves today (with the author of the first commit) if
> > you do nothing, or you can override it by dropping all but one.
> 
> I actually was (and am still) wondering that "silently ignore all of
> them if there are multiple ones that contradict with each other" is
> a bad idea, and that was why the last item on the "possible
> alternatives" list was to error out and ask clarification.

Normally I like "error out and ask the user" as an approach to avoiding
mistakes, but I can think of two bad side effects:

  1. If we pre-populate the "# Author:" lines in "git merge --squash",
     then if I run "git commit" on the result and don't explicitly take
     an action to clean up those comment fields, I get an error.  That's
     kind of annoying.

  2. Dumping the user out of "git commit" with an error isn't very
     elegant. They may have put significant work into writing the commit
     message. It's saved there in COMMIT_EDITMSG, but what is the easy
     path to them repeating their action where they left off?

It seems like the potential for confusion comes from the same place as
my complaint (1) above: the implicit-ness of the "# Author:" lines (git
writes them, assumes you've looked at and manipulated them to your
liking, and then reads them back in).

What if there was a step required of the user to say "really, I want to
use this one"? Like converting s/Author/Set-Author/, or taking away the
"#" comment character (though that has its own confusions, as you noted
earlier).

-Peff

Re: Keep original author with git merge --squash?

[David Glasser]

On Thu, Feb 12, 2015 at 2:34 PM, Junio C Hamano <gitster@pobox.com> wrote:
> David Glasser <glasser@davidglasser.net> writes:
>
>> So to be concrete: What I'm proposing (and I'm excited to implement
>> it!) is the following:
>>
>> When running "git commit" and:
>> - You've fallen into the case where the message was read from SQUASH_MSG
>> - You haven't used another method of specifying the author (--author,
>>   -C, -c, --amend)
>> - You have not specified --reset-author
>> - You have set the "commit.useSquashAuthor" option
>> - Before invoking prepare-commit-msg, all of the `Author:` lines found
>>   in SQUASH_MSG have the same value
>>
>> then that author is used, as if it were specified with --author.  (And
>> this will show up, commented-out, at the bottom of COMMIT_EDITMSG.)
>
>
> I actually was hoping that this would extend to cases other than
> "git merge --squash".
>
> When running "git commit" and:
>
>  - You didn't use a more explicit method of specifying the
>    authorship identity (--author, --date, -C, -c --amend,
>    --reset-author options, or environment variables GIT_AUTHOR_*);
>
>  - You have commit.useAuthorFromEditorComment variable;
>
>  - You have "# Author: " line that are identical in the result of
>    the editor,
>
> then use that author.  That would allow "git commit --amend" to
> update a misspelled author name, for example.
>
> Is that a bit too liberal?  Would it invite mistakes?

That does sound pretty good.  And for the specific case of squash,
what I would do would be (a) take one of the existing "Author: " lines
and turn it into an "# Author:" line and (b) do the ordinary editing
that I do to combine the existing messages?

(Maybe my proposed "detect that they're all the same" code would run
at `merge --squash` time, producing a trailing "# Author:" line if all
of the produced "Author: " lines were identical?)

--dave

-- 
glasser@davidglasser.net | langtonlabs.org | flickr.com/photos/glasser/

Re: Keep original author with git merge --squash?

[David Glasser]

On Thu, Feb 12, 2015 at 4:17 PM, David Glasser <glasser@davidglasser.net> wrote:
> On Thu, Feb 12, 2015 at 2:34 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> David Glasser <glasser@davidglasser.net> writes:
>>
>>> So to be concrete: What I'm proposing (and I'm excited to implement
>>> it!) is the following:
>>>
>>> When running "git commit" and:
>>> - You've fallen into the case where the message was read from SQUASH_MSG
>>> - You haven't used another method of specifying the author (--author,
>>>   -C, -c, --amend)
>>> - You have not specified --reset-author
>>> - You have set the "commit.useSquashAuthor" option
>>> - Before invoking prepare-commit-msg, all of the `Author:` lines found
>>>   in SQUASH_MSG have the same value
>>>
>>> then that author is used, as if it were specified with --author.  (And
>>> this will show up, commented-out, at the bottom of COMMIT_EDITMSG.)
>>
>>
>> I actually was hoping that this would extend to cases other than
>> "git merge --squash".
>>
>> When running "git commit" and:
>>
>>  - You didn't use a more explicit method of specifying the
>>    authorship identity (--author, --date, -C, -c --amend,
>>    --reset-author options, or environment variables GIT_AUTHOR_*);
>>
>>  - You have commit.useAuthorFromEditorComment variable;
>>
>>  - You have "# Author: " line that are identical in the result of
>>    the editor,
>>
>> then use that author.  That would allow "git commit --amend" to
>> update a misspelled author name, for example.
>>
>> Is that a bit too liberal?  Would it invite mistakes?
>
> That does sound pretty good.  And for the specific case of squash,
> what I would do would be (a) take one of the existing "Author: " lines
> and turn it into an "# Author:" line and (b) do the ordinary editing
> that I do to combine the existing messages?
>
> (Maybe my proposed "detect that they're all the same" code would run
> at `merge --squash` time, producing a trailing "# Author:" line if all
> of the produced "Author: " lines were identical?)

Or, well, as Peff suggested, maybe SQUASH_MSG should look exactly like
the message that rebase's squash gives you, which in fact is a lot
better: no random indent, "# Author", etc.  So making your suggested
change to parse "# Author" in `git commit`, plus a second change to
make SQUASH_MSG look like rebase's squash, would achieve my goals.


-- 
glasser@davidglasser.net | langtonlabs.org | flickr.com/photos/glasser/