git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Kristoffer Haugsbakk <code@khaugsbakk.name>
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Dragan Simic" <dsimic@manjaro.org>,
	"Manlio Perillo" <manlio.perillo@gmail.com>,
	"René Scharfe" <l.s.r@web.de>,
	"Phillip Wood" <phillip.wood@dunelm.org.uk>,
	git@vger.kernel.org
Subject: Re: [PATCH v2 16/16] config: allow multi-byte core.commentChar
Date: Fri, 15 Mar 2024 04:10:41 -0400	[thread overview]
Message-ID: <20240315081041.GA1753560@coredump.intra.peff.net> (raw)
In-Reply-To: <6be335ed-8598-406c-b535-2e58554b00e9@app.fastmail.com>

On Fri, Mar 15, 2024 at 08:16:53AM +0100, Kristoffer Haugsbakk wrote:

> > Mostly I was worried that people would take "char" in the name to assume
> > it could only be a single byte (I had originally even started the new
> > sentence with "Despite the word 'char' in the name, this option
> > can..."). And that is not just history, but a name we are stuck with
> > forever[1].
> 
> Missing footnote or referring to my footnote?
> 
> My suggestion was to use a `core.commentString` alias. Which might
> matter for new answers to questions about its use. It might not matter
> if in practice most people get their config tips from 1500 point
> StackOverflow question about how git-commit(1) keeps swallowing their
> GitHub issue numbers (due to automatic linewrap) from 2011.

Heh, missing footnote. I was going to say "we could introduce
core.commentStr or similar", but after your comment I searched in the
archive and see that you did indeed already suggest it.

I'm not sure if it would make things more or less confusing to have two
related values. One nice side effect is that the new variable would be
ignored by older versions of Git (whereas by extending core.commentChar,
you end up with config that causes older versions to barf). That
probably doesn't matter that much for most users, but as somebody who
works on Git I frequently run old versions for bug testing, bisection,
and so forth.

> > I actually do think the "string" nature is mostly uninteresting, and I'd
> > be OK leaving it as an easter egg.
> 
> To my mind a string subsumes a char (multi- or not). Like in programming
> languages: some might be used to single-char `#`, but I don’t think they
> do a double take when they see languages with `//` or `--`.

Hmm, good point. I was mostly focused on UTF-8 characters, but "//" is
quite a reasonable thing for people to try. It is probably a better
example than "foo".

> > What your suggestion doesn't say is that multi-byte characters are
> > OK. But if we think people will just assume that in a modern UTF-8
> > world, then maybe we don't need to say anything at all?
> 
> Given that we’re mostly in the context of a commit message, an
> ASCII-only restriction would feel archaic.
>
> I guess it depends on what the *normal* is in the documentation at
> large. As a user I’m used to Git handling the text that I give it.

Right, that's what I was asking. To me "character" means an ASCII byte,
but I think I might be archaic myself. ;) If most of our readers would
just assume that multi-byte characters work, perhaps it is confusing
things to even mention it.

> > It actually does not have to be UTF-8.
> 
> Good point. Unicode is more appropriate.

I think other Unicode encodings are likely to have problems (because
they embed NULs). Specifically I was thinking that you could probably
get away with latin1 or other 8-bit encodings. But again, I really hope
nobody is doing that anymore.

So anyway, adapting your original suggestion based on discussion in the
thread, maybe squash in (to the final patch):

diff --git a/Documentation/config/core.txt b/Documentation/config/core.txt
index c86b8c8408..c5a8033df9 100644
--- a/Documentation/config/core.txt
+++ b/Documentation/config/core.txt
@@ -523,9 +523,8 @@ core.commentChar::
 	Commands such as `commit` and `tag` that let you edit
 	messages consider a line that begins with this character
 	commented, and removes them after the editor returns
-	(default '#'). Note that this option can take values larger than
-	a byte (whether a single multi-byte character, or you
-	could even go wild with a multi-character sequence).
+	(default '#'). Note that this variable can be a string like
+	`//` or `⁑⁕⁑`; it doesn't have to be a single ASCII character.
 +
 If set to "auto", `git-commit` would select a character that is not
 the beginning character of any line in existing commit messages.


That's assuming we don't want to go the commentString route, which would
require a bit more re-working of the patch. I'm also open to a more
clever or pretty multi-byte example if we have one. ;)

-Peff


  reply	other threads:[~2024-03-15  8:10 UTC|newest]

Thread overview: 82+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-05  8:43 Clarify the meaning of "character" in the documentation Manlio Perillo
2024-03-05  9:00 ` Kristoffer Haugsbakk
2024-03-05 15:32   ` Junio C Hamano
2024-03-05 15:42     ` Dragan Simic
2024-03-05 16:38       ` Junio C Hamano
2024-03-05 17:28         ` Dragan Simic
2024-03-06  8:08         ` [messy PATCH] multi-byte core.commentChar Jeff King
2024-03-07  9:14           ` [PATCH 0/15] allow " Jeff King
2024-03-07  9:15             ` [PATCH 01/15] strbuf: simplify comment-handling in add_lines() helper Jeff King
2024-03-07  9:16             ` [PATCH 02/15] strbuf: avoid static variables in strbuf_add_commented_lines() Jeff King
2024-03-07  9:18             ` [PATCH 03/15] commit: refactor base-case of adjust_comment_line_char() Jeff King
2024-03-07  9:19             ` [PATCH 04/15] strbuf: avoid shadowing global comment_line_char name Jeff King
2024-03-07  9:20             ` [PATCH 05/15] environment: store comment_line_char as a string Jeff King
2024-03-07  9:21             ` [PATCH 06/15] strbuf: accept a comment string for strbuf_stripspace() Jeff King
2024-03-07  9:53               ` Jeff King
2024-03-07  9:22             ` [PATCH 07/15] strbuf: accept a comment string for strbuf_commented_addf() Jeff King
2024-03-07  9:23             ` [PATCH 08/15] strbuf: accept a comment string for strbuf_add_commented_lines() Jeff King
2024-03-07  9:23             ` [PATCH 09/15] prefer comment_line_str to comment_line_char for printing Jeff King
2024-03-07  9:24             ` [PATCH 10/15] find multi-byte comment chars in NUL-terminated strings Jeff King
2024-03-07  9:26             ` [PATCH 11/15] find multi-byte comment chars in unterminated buffers Jeff King
2024-03-07 11:08               ` Jeff King
2024-03-07 19:41                 ` René Scharfe
2024-03-07 19:47                   ` René Scharfe
2024-03-07 19:42               ` René Scharfe
2024-03-08 10:17                 ` Phillip Wood
2024-03-08 15:58                   ` Junio C Hamano
2024-03-08 16:20                     ` Phillip Wood
2024-03-12  8:19                       ` Jeff King
2024-03-12 14:36                         ` phillip.wood123
2024-03-13  6:23                           ` Jeff King
2024-03-12  8:05                 ` Jeff King
2024-03-14 19:37                   ` René Scharfe
2024-03-07  9:27             ` [PATCH 12/15] sequencer: handle multi-byte comment characters when writing todo list Jeff King
2024-03-08 10:20               ` Phillip Wood
2024-03-12  8:21                 ` Jeff King
2024-03-07  9:28             ` [PATCH 13/15] wt-status: drop custom comment-char stringification Jeff King
2024-03-07  9:30             ` [PATCH 14/15] environment: drop comment_line_char compatibility macro Jeff King
2024-03-07  9:34             ` [PATCH 15/15] config: allow multi-byte core.commentChar Jeff King
2024-03-08 11:07             ` [PATCH 0/15] " Phillip Wood
2024-03-12  9:10             ` [PATCH v2 0/16] " Jeff King
2024-03-12  9:17               ` [PATCH v2 01/16] config: forbid newline as core.commentChar Jeff King
2024-03-12  9:17               ` [PATCH v2 02/16] strbuf: simplify comment-handling in add_lines() helper Jeff King
2024-03-12  9:17               ` [PATCH v2 03/16] strbuf: avoid static variables in strbuf_add_commented_lines() Jeff King
2024-03-12  9:17               ` [PATCH v2 04/16] commit: refactor base-case of adjust_comment_line_char() Jeff King
2024-03-12  9:17               ` [PATCH v2 05/16] strbuf: avoid shadowing global comment_line_char name Jeff King
2024-03-12  9:17               ` [PATCH v2 06/16] environment: store comment_line_char as a string Jeff King
2024-03-12  9:17               ` [PATCH v2 07/16] strbuf: accept a comment string for strbuf_stripspace() Jeff King
2024-03-12  9:17               ` [PATCH v2 08/16] strbuf: accept a comment string for strbuf_commented_addf() Jeff King
2024-03-12  9:17               ` [PATCH v2 09/16] strbuf: accept a comment string for strbuf_add_commented_lines() Jeff King
2024-03-12  9:17               ` [PATCH v2 10/16] prefer comment_line_str to comment_line_char for printing Jeff King
2024-03-12  9:17               ` [PATCH v2 11/16] find multi-byte comment chars in NUL-terminated strings Jeff King
2024-03-12  9:17               ` [PATCH v2 12/16] find multi-byte comment chars in unterminated buffers Jeff King
2024-03-12  9:17               ` [PATCH v2 13/16] sequencer: handle multi-byte comment characters when writing todo list Jeff King
2024-03-12  9:17               ` [PATCH v2 14/16] wt-status: drop custom comment-char stringification Jeff King
2024-03-12  9:17               ` [PATCH v2 15/16] environment: drop comment_line_char compatibility macro Jeff King
2024-03-12  9:17               ` [PATCH v2 16/16] config: allow multi-byte core.commentChar Jeff King
2024-03-13 18:23                 ` Kristoffer Haugsbakk
2024-03-13 18:39                   ` Junio C Hamano
2024-03-15  5:59                   ` Jeff King
2024-03-15  7:16                     ` Kristoffer Haugsbakk
2024-03-15  8:10                       ` Jeff King [this message]
2024-03-15 13:30                         ` Kristoffer Haugsbakk
2024-03-15 15:40                         ` Junio C Hamano
2024-03-16  5:50                           ` Jeff King
2024-03-26 22:10                         ` Junio C Hamano
2024-03-26 22:12                           ` Kristoffer Haugsbakk
2024-03-27  7:46                           ` Jeff King
2024-03-27  8:19                             ` [PATCH 17/16] config: add core.commentString Jeff King
2024-03-27 12:45                               ` Chris Torek
2024-03-27 16:13                               ` Junio C Hamano
2024-03-28  9:47                                 ` Jeff King
2024-03-27 14:53                             ` [PATCH v2 16/16] config: allow multi-byte core.commentChar Junio C Hamano
2024-03-12 14:40               ` [PATCH v2 0/16] " phillip.wood123
2024-03-12 20:30                 ` Junio C Hamano
2024-03-05 16:58       ` Clarify the meaning of "character" in the documentation Kristoffer Haugsbakk
2024-03-05 17:20         ` Dragan Simic
2024-03-05 17:37           ` Kristoffer Haugsbakk
2024-03-05 21:19             ` Dragan Simic
2024-03-05 16:51     ` Kristoffer Haugsbakk
2024-03-05 17:37       ` Junio C Hamano
2024-03-05 17:49         ` Kristoffer Haugsbakk
2024-03-05 22:48   ` brian m. carlson

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=20240315081041.GA1753560@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=code@khaugsbakk.name \
    --cc=dsimic@manjaro.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=l.s.r@web.de \
    --cc=manlio.perillo@gmail.com \
    --cc=phillip.wood@dunelm.org.uk \
    /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).