All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Phillip Wood <phillip.wood123@gmail.com>
Cc: Brian Lyles <brianmlyles@gmail.com>,  git@vger.kernel.org
Subject: Re: [PATCH v2] interpret-trailers: handle message without trailing newline
Date: Fri, 06 Sep 2024 08:23:42 -0700	[thread overview]
Message-ID: <xmqqmskkyfep.fsf@gitster.g> (raw)
In-Reply-To: <fab48d5b-4808-439e-9384-ca4861b95edc@gmail.com> (Phillip Wood's message of "Fri, 6 Sep 2024 10:07:02 +0100")

Phillip Wood <phillip.wood123@gmail.com> writes:

> Thanks for the comprehensive commit message. If the problem only affects
> "git interpret-trailers" I wonder if it would be simpler to do
>
> diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
> index 1d969494cf..e6f22459f1 100644
> --- a/builtin/interpret-trailers.c
> +++ b/builtin/interpret-trailers.c
> @@ -132,6 +132,7 @@ static void read_input_file(struct strbuf *sb, const char *file)
>                  if (strbuf_read(sb, fileno(stdin), 0) < 0)
>                          die_errno(_("could not read from stdin"));
>          }
> +        strbuf_complete_line(sb);
>  }

It is much simpler, and if we are to require a message that the user
uses interpret-trailers on not to end in an incomplete line (which I
do not have any objection to), it is absolutely the right approach.

With a devil's advocate hat on, though, if the trailer operation is
to find the trailer on the incomplete line at the end, and insert a
trailer _before_ that one, would it be more faithful to the command
given by the end-user, if we inserted the new trailer without
touching the existing trailer line (including its lack of
terminating EOL)?  Which would mean that we'd need to remember the
fact that we added a LF here, and then before writing the result out
make the buffer to end with an incomplete line.  Which I personally
think is crazy, compared to the approach to declare that a message
that you subject to interpret-trailers command MUST BE a proper
text, not ending with an incomplete line.

So, yeah, I like that idea.


  reply	other threads:[~2024-09-06 15:23 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-05 17:34 [PATCH] interpret-trailers: handle message without trailing newline Brian Lyles
2024-09-05 18:24 ` Brian Lyles
2024-09-06  4:08 ` [PATCH v2] " Brian Lyles
2024-09-06  9:07   ` Phillip Wood
2024-09-06 15:23     ` Junio C Hamano [this message]
2024-09-06 14:50 ` [PATCH v3] " Brian Lyles
2024-09-06 16:21   ` Junio C Hamano
2024-09-09  9:13     ` Phillip Wood
2024-09-09 15:58       ` Junio C Hamano
2024-09-09  9:13   ` Phillip Wood

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

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqmskkyfep.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=brianmlyles@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=phillip.wood123@gmail.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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.