[prev in list] [next in list] [prev in thread] [next in thread] 

List:       git
Subject:    Re: [PATCH 2/2] commit: fix ending newline for template files
From:       Eric Sunshine <sunshine () sunshineco ! com>
Date:       2015-05-31 2:21:53
Message-ID: CAPig+cTd9OjXkJY3=gQ5b8ZJqLEubhBEN_xm_i1g6CNUxNo1CQ () mail ! gmail ! com
[Download RAW message or body]

On Sat, May 30, 2015 at 7:29 AM, Patryk Obara <patryk.obara@gmail.com> wrote:
> On Thu, May 28, 2015 at 4:29 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>> Did you consider the alternate approach of handling newline processing
>> immediately upon loading 'logfile' and 'template_file', rather than
>> delaying processing until this point? Doing it that way would involve
>> a bit of code repetition but might be easier to reason about since it
>> would occur before possible interactions in following code (such as
>> --signoff handling).
>
> Yes. I opted to place it in here, because newline was appended previously
> also in "if (use_editor)" block. But I agree, appending this newline after
> loading file will be cleaner - and code repetition may be avoided, if I'll
> separate file loading code into new function.

A need for this sort of functionality has come up before, so it might
be reasonable to introduce a new strbuf function for appending a
character if missing. In addition to the 'newline' case, appending '/'
to a pathname is also somewhat common.

> On Sat, May 30, 2015 at 12:25 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>> If the user specified with the --cleanup option not to
>> clean-up the result coming back from the editor, then the commented
>> material needs to be removed in the editor by the user *anyway*.

You misattributed this statement. It was from Junio, not I.

> Why? Is it not ok to leave lines starting with hash in commit object?
> --cleanup=whitespace|verbatim suggests, that it's a valid usecase.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
[prev in list] [next in list] [prev in thread] [next in thread]