From: Junio C Hamano Date: Fri, 21 Oct 2016 00:18:49 GMT Subject: Re: [PATCH v4 5/8] trailer: clarify failure modes in parse_trailer Message-ID: In-Reply-To: <5656444f-14ba-b58c-4ad6-e72cc15a13cb@google.com> Jonathan Tan 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 " 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. [Footnote] *1* Or more lenient "[A-Za-z0-9][- A-Za-z0-9]*:".