git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Christian Couder <christian.couder@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Jonathan Tan <jonathantanmy@google.com>,
	Stefan Beller <sbeller@google.com>,
	"git@vger.kernel.org" <git@vger.kernel.org>,
	Ramsay Jones <ramsay@ramsayjones.plus.com>
Subject: Re: [PATCH v4 5/8] trailer: clarify failure modes in parse_trailer
Date: Sat, 22 Oct 2016 15:07:25 +0200	[thread overview]
Message-ID: <CAP8UFD0XvNx6Brio9muDhOj5+dr=HRbhK_Lmr+k4LUPKrAVC9Q@mail.gmail.com> (raw)
In-Reply-To: <xmqqeg3aeeqe.fsf@gitster.mtv.corp.google.com>

On Fri, Oct 21, 2016 at 2:18 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Jonathan Tan <jonathantanmy@google.com> writes:
>
>> That is true - I think we can take the allowed separators as an
>> argument (meaning that we can have different behavior for file parsing
>> and command line parsing), and since we already have that string, we
>> can use strcspn. I'll try this out in the next reroll.
>
> Sounds good.  Thanks.
>
>
> The following is a tangent that I think this topic should ignore,
> but we may want to revisit it sometime later.
>
> I think the design of the "separator" mechanism is one of the things
> we botched in the current system.  If I recall correctly, this was
> introduced to allow people write "Bug# 538" in the trailer section
> and get it recognised as a valid trailer.
>
> When I say that this was a botched design, I do not mean to say that
> we should have instead forced projects to adopt "Bug: 538" format.
> The design is botched because the users' wish to allow "Bug# 538" or
> "Bug #538" by setting separators to ":#" from the built-in ":" does
> not mean that they would want "Signed-off-by# me <my@addre.ss>" to
> be accepted.
>
> If I were guiding a topic that introduce this feature from scratch
> today, I would probably suggest a pattern based approach, e.g.  a
> built-in "[-A-Za-z0-9]+:" [*1*] may be the default prefix that is
> used to recognize the beginning of a trailer, and a user or a
> project that wants "Bug #538" would be allowed to add an additional
> pattern, e.g. "Bug *#", that recognises a custom trailer line that
> is used by the project.

When we designed the separator mechanism, we had the following discussions:

https://public-inbox.org/git/xmqqa9a1d6xn.fsf@gitster.dls.corp.google.com/
https://public-inbox.org/git/xmqqmwcuzyqx.fsf@gitster.dls.corp.google.com/

They made me think that you were against too much flexibility, so I
removed functionality that allowed to put separators into the ".key"
config options, and now you are saying that we botched the thing and
that you would like more flexibility of this kind back.

Anyway I think it is still possible to add back such kind of
functionality in a backward compatible way for example by adding
".extendedKey" config options.

  reply	other threads:[~2016-10-22 13:07 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-12  1:23 [PATCH 0/5] allow non-trailers and multiple-line trailers Jonathan Tan
2016-10-12  1:23 ` [PATCH 1/5] trailer: use singly-linked list, not doubly Jonathan Tan
2016-10-12  6:24   ` Junio C Hamano
2016-10-12 15:38   ` Christian Couder
2016-10-12 17:26     ` Jeff King
2016-10-12  1:23 ` [PATCH 2/5] trailer: streamline trailer item create and add Jonathan Tan
2016-10-12  1:23 ` [PATCH 3/5] trailer: make args have their own struct Jonathan Tan
2016-10-12  1:23 ` [PATCH 4/5] trailer: allow non-trailers in trailer block Jonathan Tan
2016-10-12  1:23 ` [PATCH 5/5] trailer: support values folded to multiple lines Jonathan Tan
2016-10-12  6:23   ` Junio C Hamano
2016-10-12 23:40 ` [PATCH v2 0/6] allow non-trailers and multiple-line trailers Jonathan Tan
2016-10-12 23:40 ` [PATCH v2 1/6] trailer: improve const correctness Jonathan Tan
2016-10-12 23:40 ` [PATCH v2 2/6] trailer: use list.h for doubly-linked list Jonathan Tan
2016-10-14 17:29   ` Jakub Narębski
2016-10-14 18:27   ` Junio C Hamano
2016-10-12 23:40 ` [PATCH v2 3/6] trailer: streamline trailer item create and add Jonathan Tan
2016-10-12 23:40 ` [PATCH v2 4/6] trailer: make args have their own struct Jonathan Tan
2016-10-12 23:40 ` [PATCH v2 5/6] trailer: allow non-trailers in trailer block Jonathan Tan
2016-10-12 23:40 ` [PATCH v2 6/6] trailer: support values folded to multiple lines Jonathan Tan
2016-10-14 17:37 ` [PATCH v3 0/6] allow non-trailers and multiple-line trailers Jonathan Tan
2016-10-14 17:37 ` [PATCH v3 1/6] trailer: improve const correctness Jonathan Tan
2016-10-17 22:49   ` Stefan Beller
2016-10-14 17:37 ` [PATCH v3 2/6] trailer: use list.h for doubly-linked list Jonathan Tan
2016-10-14 17:38 ` [PATCH v3 3/6] trailer: streamline trailer item create and add Jonathan Tan
2016-10-17 23:01   ` Stefan Beller
2016-10-14 17:38 ` [PATCH v3 4/6] trailer: make args have their own struct Jonathan Tan
2016-10-17 23:20   ` Stefan Beller
2016-10-18 16:05     ` Junio C Hamano
2016-10-14 17:38 ` [PATCH v3 5/6] trailer: allow non-trailers in trailer block Jonathan Tan
2016-10-18  0:49   ` Stefan Beller
2016-10-18  1:42     ` Junio C Hamano
2016-10-18  2:02       ` Jonathan Tan
2016-10-18 16:36         ` Junio C Hamano
2016-10-19 18:00           ` Jonathan Tan
2016-10-19 19:24             ` Junio C Hamano
2016-10-14 17:38 ` [PATCH v3 6/6] trailer: support values folded to multiple lines Jonathan Tan
2016-10-18  0:55   ` Stefan Beller
2016-10-20 21:39 ` [PATCH v4 0/8] allow non-trailers and multiple-line trailers Jonathan Tan
2016-10-20 21:39 ` [PATCH v4 1/8] trailer: improve const correctness Jonathan Tan
2016-10-20 21:39 ` [PATCH v4 2/8] trailer: use list.h for doubly-linked list Jonathan Tan
2016-10-20 21:39 ` [PATCH v4 3/8] trailer: streamline trailer item create and add Jonathan Tan
2016-10-20 21:39 ` [PATCH v4 4/8] trailer: make args have their own struct Jonathan Tan
2016-10-20 21:39 ` [PATCH v4 5/8] trailer: clarify failure modes in parse_trailer Jonathan Tan
2016-10-20 22:07   ` Stefan Beller
2016-10-20 22:14     ` Junio C Hamano
2016-10-20 22:40       ` Jonathan Tan
2016-10-20 22:45         ` Junio C Hamano
2016-10-20 22:49           ` Jonathan Tan
2016-10-21  0:18             ` Junio C Hamano
2016-10-22 13:07               ` Christian Couder [this message]
2016-10-22 16:19                 ` Junio C Hamano
2016-10-22  9:29           ` Christian Couder
2016-10-20 22:45         ` Jonathan Tan
2016-10-20 21:39 ` [PATCH v4 6/8] trailer: allow non-trailers in trailer block Jonathan Tan
2016-10-20 21:39 ` [PATCH v4 7/8] trailer: forbid leading whitespace in trailers Jonathan Tan
2016-10-20 21:39 ` [PATCH v4 8/8] trailer: support values folded to multiple lines Jonathan Tan
2016-10-21 17:54 ` [PATCH v5 0/8] allow non-trailers and multiple-line trailers Jonathan Tan
2016-10-21 23:59   ` Junio C Hamano
2016-10-22  0:06     ` Stefan Beller
2016-10-21 17:54 ` [PATCH v5 1/8] trailer: improve const correctness Jonathan Tan
2016-10-21 17:54 ` [PATCH v5 2/8] trailer: use list.h for doubly-linked list Jonathan Tan
2016-10-21 17:54 ` [PATCH v5 3/8] trailer: streamline trailer item create and add Jonathan Tan
2016-10-21 17:54 ` [PATCH v5 4/8] trailer: make args have their own struct Jonathan Tan
2016-10-21 17:55 ` [PATCH v5 5/8] trailer: clarify failure modes in parse_trailer Jonathan Tan
2016-10-21 17:55 ` [PATCH v5 6/8] trailer: allow non-trailers in trailer block Jonathan Tan
2016-10-21 17:55 ` [PATCH v5 7/8] trailer: forbid leading whitespace in trailers Jonathan Tan
2016-10-21 17:55 ` [PATCH v5 8/8] trailer: support values folded to multiple lines Jonathan Tan

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='CAP8UFD0XvNx6Brio9muDhOj5+dr=HRbhK_Lmr+k4LUPKrAVC9Q@mail.gmail.com' \
    --to=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jonathantanmy@google.com \
    --cc=ramsay@ramsayjones.plus.com \
    --cc=sbeller@google.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).