Re: [PATCH 1/2] commit: do not cleanup template

From: Boris 'billiob' Faure <billiob@gmail.com>
Date: May 7, 2011, 21:17 UTC
Message-ID: <BANLkTinvwjoa4z2JEiuPD44BV2V_m50VYQ@mail.gmail.com>
In-Reply-To: <7vwri2uv4m.fsf@alter.siamese.dyndns.org>

On Sat, May 7, 2011 at 22:13, Junio C Hamano <gitster@pobox.com> wrote:

Show 9 quoted lines

> […]
> My gut feeling is that it is relatively easy to justify [1/2]. We prepare
> the message file to be edited by the committer in sb by reading from
> different message sources, and usually we would want to clean up what is
> in sb before writing it out, but you may want to enforce a particular
> format by using the template (such as having a leading whitespace) and the
> way to tweak and fix that appearance should be by editing the template,
> not by automatically running stripspace() here [*1*] when the message came
> from the template.

That's exactly my point of view.

I use the commit template feature mostly to show additional pieces of information I'd use in my editor when writing the log. It had scratched an itch because I have to insert manually a newline to write the commit title whenever I use a commit log template. It's a bit strange for me to have that different behavior when writing a commit log whether the commit log template feature is used or not. I consider the newline being part of the template since sometimes the template could even pre-fill the commit log title. That's the main reason I disabled the cleanup when using a template.

> I cannot tell offhand how you justify [2/2], though. What motivated you to
> remove this blank line? At this point in the codepath, it does not look
> like it should matter if the original message came from your template or
> from somewhere else.

In the way I use the commit log template, I consider that new line inserted there as useless.

> If the blank line is unneeded after "# You may be committing a merge..."
> for readability, wouldn't that blank line be unneeded when you took the
> message from other places, no?

Maybe.

> It might make sense to move that newline before "if (whence != FROM_COMMIT)"
> block, though, to make the logic easier to follow, regardless of the use
> of the template file.

Moving the newline there will not change my point of view on the usefulness of it when using a template.

-- 
Boris Faure