git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2] trailer: allow spaces in tokens

From
Christian Couder <christian.couder@gmail.com>
Date
Aug 18, 2022, 20:46 UTC
Message-ID
<CAP8UFD1304rQh+9jMUkTmTvmNBxm6BNDt3=d07+C8iXSbJb2RA@mail.gmail.com>
In-Reply-To
<CM9DZ4Z18PY9.1T2O8U54X66FD@cedar>
On Thu, Aug 18, 2022 at 9:03 PM Maxwell Bernstein <tekk.nolagi@gmail.com> wrote:
Show 21 quoted lines
>
> On Thu Aug 18, 2022 at 9:54 AM PDT, Junio C Hamano wrote:
> > So, the updated code mistook the body of the message that is not a
> > sign-off, because there is a colon on the line, the line does not
> > begin with the colon, and everything before the colon is an alnum or
> > a whitespace, so squashed the paragraph break before the real
> > trailer block and the last line of the body and made it a body-less
> > commit log message?
>
> I agree that it is not ideal, and I'm not sure how to fix it. This commit
> probably shouldn't go in as-is; I modified the test in order to demonstrate
> this problem.
>
> > This might be a good demonstration of why it is a mistaken design to
> > allow whitespaces, which may steer us toward fixing the documentation?
> >
> > I dunno.  What do others think?
>
> I think allowing whitespace is good at least for the Phabricator project, which
> despite being dead, has a number of existing users and existing commits. It
> unfortunately has a "Differential revision" trailer with whitespace.

I think allowing one white space before the separator is a good idea for the following reasons.

First, if people use something like #, !, ~ or % as a separator, for example in the case of a trailer like "Bug #43", it is very natural to have a white space before the # separator.

Note that GitLab for example uses the ~N format, where N is a number, for issues, !N for merge requests and %N for milestones. I think Bugzilla and many other bug trackers use a #N format for bug numbers.

Also in some languages, like French, the typographical rules when writing regular text is to have a space (technically it's supposed to be a "non breaking space", see: https://en.wikipedia.org/wiki/Non-breaking_space, but in practice people use a regular space most of the time) before a colon. So it is very natural for a number of people in the world to automatically add a white space before a colon.

https://en.wikipedia.org/wiki/Colon_(punctuation)#Spacing says: "In print, a thin space was traditionally placed before a colon and a thick space after it. In modern English-language printing, no space is placed before a colon and a single space is placed after it. In French-language typing and printing, the traditional rules are preserved. "

I think it would be very annoying for users to find out that a number of otherwise proper trailers would not be taken into account only because they have a white space before the colon. At least there should be an option to allow that.

Previous: Maxwell BernsteinNext: Junio C Hamano
Message 6 of 19 in “trailer: allow spaces in tokens”
  1. trailer: allow spaces in tokensMax Bernstein via GitGitGadget, Aug 18, 2022
  2. trailer: allow spaces in tokensMax Bernstein via GitGitGadget, Aug 18, 2022
  3. Junio C HamanoAug 18, 2022
  4. Jonathan TanAug 18, 2022
  5. Maxwell BernsteinAug 18, 2022
  6. Christian CouderAug 18, 2022
  7. Junio C HamanoAug 18, 2022
  8. Junio C HamanoAug 19, 2022
  9. Christian CouderAug 19, 2022
  10. Johannes SchindelinAug 22, 2022
  11. Documentation: clarify whitespace rules for trailersChristian Couder, Aug 23, 2022
  12. Junio C HamanoAug 24, 2022
  13. Christian CouderAug 25, 2022
  14. Junio C HamanoAug 25, 2022
  15. Documentation: clarify whitespace rules for trailersChristian Couder, Aug 30, 2022
  16. Junio C HamanoAug 30, 2022
  17. Junio C HamanoAug 18, 2022
  18. Jonathan TanAug 18, 2022
  19. Junio C HamanoAug 18, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.