From: Jonathan Tan <jonathantanmy@google.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, Christian Couder <christian.couder@gmail.com>
Subject: Re: [RFC/PATCH 0/2] place cherry pick line below commit title
Date: Mon, 3 Oct 2016 14:28:14 -0700 [thread overview]
Message-ID: <d3df0636-1975-1d08-2f34-384984c72e5d@google.com> (raw)
In-Reply-To: <xmqqbmz16y42.fsf@gitster.mtv.corp.google.com>
On 10/03/2016 12:17 PM, Junio C Hamano wrote:
> It may be necessary for the code to analize the lines in a block
> identified as "likely to be a trailing block" more carefully,
> though. The example 59f0aa94 in the message you are responding to
> has "Link 1:" that consists of 3 physical lines. An instruction to
> interpret-trailers to add a new one _after_ "Link-$n:" may have to
> treat these as a single logical line and do the addition after them,
> i.e. before "Link 2:" line, for example.
>
> I also saw
>
> Signed-off-by: Some body <some@body.xz> (some comment
> that extends to the next line without being indented)
> Sined-off-by: Some body Else <some.body@else.xz>
>
> where the only clue that the second line is logically a part of the
> first one was the balancing of parentheses (or [brakets]). To
> accomodate real-world use cases, you may have to take into account a
> lot more than the strict rfc-822 style "line folding".
If we define a logical trailer line as the set of physical lines until
the next trailer line...it is true that this has some precedence in that
such lines can be written (although this might not be intentional):
$ git interpret-trailers --trailer "a=foo
bar" </dev/null
a: foo
bar
This solves the "Line 1:" case above, but raises other issues:
1. Checking for existence (trailer.ifexists) is now conceptually more
difficult. For example, handling of inner whitespace in between lines
might be confusing (currently, there is only one line, and whitespace
handling is clearly described).
2. Replacement (trailer.ifexists=replace) might replace more than expected.
3. There is a corner case - the first line might not be a trailer line.
Even if interpret-trailers knew about "(cherry picked from", a user
might enter something else there. (We could just declare such blocks as
non-trailers, but if we are already loosening the definition of a
trailer, this might be something that we should allow.)
My initial thought was to think of a trailer as a block of trailer lines
possibly interspersed with other lines. This leads to interpret-trailers
placing the trailer in the wrong place if trailer.where=after, as you
describe, but this might not be a big problem if trailer.where=after is
not widely used (and it is not the default). (The other options behave
as expected.)
There are other options like checking for indentation or checking for
balanced parentheses/brackets, but I think that these would lead to
surprising behavior for the user (this would mean that whitespace or
certain characters could turn a valid trailer into an invalid one or
vice versa, or change the behavior of trailer.ifexists, especially
"replace").
next prev parent reply other threads:[~2016-10-03 21:28 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-29 19:21 [RFC/PATCH 0/2] place cherry pick line below commit title Jonathan Tan
2016-09-29 19:21 ` [RFC/PATCH 1/2] sequencer: refactor message and origin appending Jonathan Tan
2016-09-29 19:21 ` [RFC/PATCH 2/2] sequencer: allow origin line below commit title Jonathan Tan
2016-09-29 21:56 ` [RFC/PATCH 0/2] place cherry pick " Junio C Hamano
2016-09-30 18:22 ` Jonathan Tan
2016-09-30 19:34 ` Junio C Hamano
2016-09-30 20:23 ` Jonathan Tan
2016-10-03 15:23 ` Junio C Hamano
2016-09-30 20:49 ` Junio C Hamano
2016-10-03 17:44 ` Jonathan Tan
2016-10-03 19:17 ` Junio C Hamano
2016-10-03 21:28 ` Jonathan Tan [this message]
2016-10-03 22:13 ` Junio C Hamano
2016-10-04 0:08 ` Jonathan Tan
2016-10-04 17:25 ` Junio C Hamano
2016-10-04 18:28 ` Junio C Hamano
2016-10-05 19:44 ` Jonathan Tan
2016-10-06 19:24 ` Junio C Hamano
2016-10-05 19:38 ` Jonathan Tan
2016-10-05 20:33 ` Junio C Hamano
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=d3df0636-1975-1d08-2f34-384984c72e5d@google.com \
--to=jonathantanmy@google.com \
--cc=christian.couder@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.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).