From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: Ross Kabus <rkabus@aerotech.com>, git@vger.kernel.org
Subject: Re: [Bug] commit-tree shouldn't append an extra newline to commit messages
Date: Wed, 06 Sep 2017 06:56:43 +0900 [thread overview]
Message-ID: <xmqq3780byhw.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20170905170311.yhcksrw2bxevd3hk@sigill.intra.peff.net> (Jeff King's message of "Tue, 5 Sep 2017 13:03:12 -0400")
Jeff King <peff@peff.net> writes:
> On Tue, Sep 05, 2017 at 12:57:21PM -0400, Ross Kabus wrote:
>
>> On Tue, Sep 5, 2017 at 11:36 AM, Jeff King <peff@peff.net> wrote:
>>
>> > So I'd argue that "git commit -F" is doing a reasonable
>> > thing, and "commit-tree -F" should probably change to match it (because
>> > it's inconsistent, and because if anything the plumbing commit-tree
>> > should err more on the side of not touching its input than the porcelain
>> > commit command).
>>
>> I would agree that "commit-tree -F" should change to match the behavior of
>> "git commit -F --cleanup=verbatim". I feel pretty strongly that this type of
>> cleanup logic shouldn't be done in a plumbing command, though I'm not sure
>> it is a big enough deal to change behavior/compatibility for everyone.
>
> OK. Do you want to try your hand at a patch?
>
>> Yup, confusion #2. I was using "-F -" which I see now is a different code
>> path. Reading via stdin without "-F -" _is_ the verbatim option. This
>> difference burned someone else on the mailing list as well. See:
>
> Ah, OK, your confusion makes more sense now.
Yeah, my initial knee-jerk reaction when I started reading the early
part of the thread was that the use of strbuf_complete_line() is a
strong enough sign that this was intended behaviour, but the "we get
different results between reading from standard input and pretending
'-' is a file and reading from it" made me change my mind. I agree
that dropping strbuf_complete_line() call from that codepath (and
nowhere else, especially the "-m" oneline thing [*1*]) would be a
backward incompatible change that is acceptable.
Here is what I am preparing to add to the What's cooking report when
such a patch materializes ;-)
* "git commit-tree -F $file" used to add terminating newline if the
input ends with an incomplete line, but when the command reads
the input from its standard input, we recorded what was given
verbatim. The former has been changed to record the input with
an incomplete line to make them consistent.
Something like that probably needs to be added to the release notes
but I am way being ahead of myself ;-)
Thanks, both, for sensible discussion.
[Footnote]
*1* The message from Ross you are responding to talks about
"cleanup", but I tend to view the calls to *_complete_line() as
more for "the end-user convenience". "-m" meant to take a
one-liner from the command line should have it because exactly
as you said, having to pass the terminating newline from the
command line with "-m" is awkward. On the other hand, if the
user/caller prepared the exact message in a file and wants to
feed it either via "-F" or from the standard input, I agree that
"the end-user convenience" would get in the way and it is
preferrable to avoid it in the plumbing layer.
prev parent reply other threads:[~2017-09-05 21:56 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-01 18:58 [Bug] commit-tree shouldn't append an extra newline to commit messages Ross Kabus
2017-09-02 8:33 ` Jeff King
2017-09-05 15:09 ` Ross Kabus
2017-09-05 15:36 ` Jeff King
2017-09-05 16:57 ` Ross Kabus
2017-09-05 17:03 ` Jeff King
2017-09-05 20:57 ` Ross Kabus
2017-09-05 20:59 ` Ross Kabus
2017-09-07 5:57 ` Jeff King
2017-09-05 21:56 ` Junio C Hamano [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqq3780byhw.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=peff@peff.net \
--cc=rkabus@aerotech.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).