On Fri, Feb 15, 2013 at 10:58:38AM -0800, Brandon Casey wrote: > On Thu, Feb 14, 2013 at 9:58 AM, John Keeping <john@xxxxxxxxxxxxx> wrote: > > As Jonathan Nieder wondered before [1], this changes the behaviour when > > the commit message is empty. Before this commit, there is an empty line > > followed by the S-O-B line; now the S-O-B is on the first line of the > > commit. > > > > The previous behaviour seems better to me since the empty line is > > hinting that the user should fill something in. It looks particularly > > strange if your editor has syntax highlighting for commit messages such > > that the first line is in a different colour. > > Are you talking about the output produced by format-patch? Or are you > talking about what happens when you do 'commit --amend -s' for a > commit with an empty commit message. (The email that you referenced > was about the behavior of format-patch). I'm talking about plain 'commit -s' which seems to use the same code path. > I'm thinking you must be talking about the 'commit --amend -s' > behavior since you mentioned your editor. Is there another case that > is affected by this? Normally, any extra blank lines that precede or > follow a commit message are removed before the commit object is > created. So, I guess it wouldn't hurt to insert a newline (or maybe > it should be two?) before the signoff in this case. Would this > provide an improvement or change for any other commands than 'commit > --amend -s'? > > If we want to do this, then I'd probably do it like this: > > - if (len && msgbuf->buf[len - 1] != '\n') > + if (!len || msgbuf->buf[len - 1] != '\n') > append_newlines = "\n\n"; > - else if (len > 1 && msgbuf->buf[len - 2] != '\n') > + else if (len == 1 || msgbuf->buf[len - 2] != '\n') > append_newlines = "\n"; > > This would ensure there were two newlines preceding the sob. The > editor would place its cursor on the top line where the user should > begin typing in a commit message. If an editor was not opened up > (e.g. if 'git cherry-pick -s --allow-empty-message ...' was used) then > the normal mechanism that removes extra blank lines would trigger to > remove the extra blank lines. > > I think that's reasonable. Two blank lines seems like an improvement to me, FWIW. John -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html