Re: [PATCH 2/2] merge: make sure to terminate message with newline

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> I think we still end up calling cleanup_message() on the result before
> using it as the final message, and that will fix any missing newline.
> But we feed the intermediate state before then to the hooks (which is
> exactly where one might expect to use interpret-trailers).
>
> I'm not sure if we should be doing a preemptive call to
> cleanup_message() before feeding the hooks (we'd still need to do the
> final one, to clean up whatever the hooks return to us). I guess
> probably not, because I think that would remove comments, as well. So
> adding in just the missing newline is probably better.

To be quite honest, I think this patch is a half-way solution and I
am not sure if it is better than either of the two "purist"
extremes:

 * If the hooks want to see messages with as little loss of
   information from the original as possible, we should give them
   without clean-up (which you pointed out above) *and* without this
   patch.

 * If the hooks want to see messages as canonicalized as people
   would see in normal "git log" output, we should be passing the
   full clean-up to lose even comments and in such a case there is
   no need for this "always terminate" patch (we'd instead do far
   more).

Between the two approaches, both are purists' view, I'd prefer the
former, but from that stance, the conclusion would become that there
is no need to do anything, which may be a bit unsatisfactory.

> Since it will usually be added back in by cleanup_message() anyway, I
> think it's OK to just add it preemptively. The exception would be if the
> user asked for no cleanup at all. So making it conditional on
> cleanup_mode would work, whether -F or not.
>
> I suppose that does mean people turning off cleanup mode would get a
> message without a newline from fmt_merge_msg(), though, which is perhaps
> unexpected.
>
> So maybe just keeping the newline there, as you suggest, is the best
> way.

Hmph.

If the user tells us "refrain from touching my message as much as
possible" and feeds us a proposed log message that ends with an
incomplete line, I would think they expect us not to turn it into a
complete line, and I would think doing this change only when cleanup
is in effect would make sense.  This is especially true for users
who do not let any hooks to interfere.  They used to get their
incomplete lines intact, now their incomplete lines will
unconditionally get completed.  I am not sure if I would want to
defend this change from their complaints.