All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: git@vger.kernel.org
Cc: rsbecker@nexbridge.com,  Dragan Simic <dsimic@manjaro.org>,
	 Chris Torek <chris.torek@gmail.com>,
	 Ralph Seichter <github@seichter.de>,
	 "Ralph Seichter via GitGitGadget" <gitgitgadget@gmail.com>
Subject: [PATCH 3/1] config: allow tweaking whitespace between value and comment
Date: Fri, 15 Mar 2024 15:26:40 -0700	[thread overview]
Message-ID: <xmqq4jd7p1wf.fsf_-_@gitster.g> (raw)
In-Reply-To: <xmqq8r2jp2eq.fsf@gitster.g> (Junio C. Hamano's message of "Fri, 15 Mar 2024 15:15:41 -0700")

Extending the previous step, this allows the whitespace placed after
the value before the "# comment message" to be tweaked by tweaking
the preprocessing rule to:

 * If the given comment string begins with one or more whitespace
   characters followed by '#', it is passed intact.

 * If the given comment string begins with '#', a Space is
   prepended.

 * Otherwise, " # " (Space, '#', Space) is prefixed.

 * A string with LF in it cannot be used as a comment string.

Unlike the previous step, which unconditionally added a space after
the value before writing the "# comment string", because the above
preprocessing already gives a whitespace before the '#', the
resulting string is written immediately after copying the value.

And the sanity checking rule becomes

 * comment string after the above massaging that comes into
   git_config_set_multivar_in_file_gently() must

   - begin with zero or more whitespace characters followed by '#'.
   - not have a LF in it.

I personally think this is over-engineered, but since I thought
things through anyway, here it is in the patch form.  The logic to
tweak end-user supplied comment string is encapsulated in a new
helper function, git_config_prepare_comment_string(), so if new
front-end callers would want to use the same massaging rules, it is
easily reused.

Unfortunately I do not think of a way to tweak the preprocessing
rules further to optionally allow having no blank after the value,
i.e. to produce

	[section]
		variable = value#comment

(which is a valid way to say section.variable=value, by the way)
without sacrificing the ergonomics for the more usual case, so this
time I really stop here.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * This is [3/1] as the one attached to my review of the single
   patch is [2/1].

 Documentation/git-config.txt | 12 +++++--
 builtin/config.c             |  7 +---
 config.c                     | 69 ++++++++++++++++++++++++++++++------
 config.h                     |  2 ++
 t/t1300-config.sh            |  6 ++++
 5 files changed, 76 insertions(+), 20 deletions(-)

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index af374ee2e0..e4f2e07926 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -89,9 +89,15 @@ OPTIONS
 
 --comment <message>::
 	Append a comment at the end of new or modified lines.
-	Unless _<message>_ begins with "#", a string "# " (hash
-	followed by a space) is prepended to it. The _<message>_ must not
-	contain linefeed characters (no multi-line comments are permitted).
+
+	If _<message>_ begins with one or more whitespaces followed
+	by "#", it is used as-is.  If it begins with "#", a space is
+	prepended before it is used.  Otherwise, a string " # " (a
+	space followed by a hash followed by a space) is prepended
+	to it.  And the resulting string is placed immediately after
+	the value defined for the variable.  The _<message>_ must
+	not contain linefeed characters (no multi-line comments are
+	permitted).
 
 --get::
 	Get the value for a given key (optionally filtered by a regex
diff --git a/builtin/config.c b/builtin/config.c
index e859a659f4..0015620dde 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -841,12 +841,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		flags |= CONFIG_FLAGS_FIXED_VALUE;
 	}
 
-	if (comment) {
-		if (strchr(comment, '\n'))
-			die(_("no multi-line comment allowed: '%s'"), comment);
-		if (comment[0] != '#')
-			comment = xstrfmt("# %s", comment);
-	}
+	comment = git_config_prepare_comment_string(comment);
 
 	if (actions & PAGING_ACTIONS)
 		setup_auto_pager("config", 1);
diff --git a/config.c b/config.c
index 15019cb9a5..f1d4263a68 100644
--- a/config.c
+++ b/config.c
@@ -3044,7 +3044,7 @@ static ssize_t write_pair(int fd, const char *key, const char *value,
 		}
 
 	if (comment)
-		strbuf_addf(&sb, "%s %s\n", quote, comment);
+		strbuf_addf(&sb, "%s%s\n", quote, comment);
 	else
 		strbuf_addf(&sb, "%s\n", quote);
 
@@ -3172,6 +3172,62 @@ void git_config_set(const char *key, const char *value)
 	trace2_cmd_set_config(key, value);
 }
 
+/*
+ * The ownership rule is that the caller will own the string
+ * if it receives a piece of memory different from what it passed
+ * as the parameter.
+ */
+const char *git_config_prepare_comment_string(const char *comment)
+{
+	size_t leading_blanks;
+
+	if (!comment)
+		return NULL;
+
+	if (strchr(comment, '\n'))
+		die(_("no multi-line comment allowed: '%s'"), comment);
+
+	/*
+	 * If it begins with one or more leading whitespace characters
+	 * followed by '#", the comment string is used as-is.
+	 *
+	 * If it begins with '#', a SP is inserted between the comment
+	 * and the value the comment is about.
+	 *
+	 * Otherwise, the value is followed by a SP followed by '#'
+	 * followed by SP and then the comment string comes.
+	 */
+
+	leading_blanks = strspn(comment, " \t");
+	if (leading_blanks && comment[leading_blanks] == '#')
+		; /* use it as-is */
+	else if (comment[0] == '#')
+		comment = xstrfmt(" %s", comment);
+	else
+		comment = xstrfmt(" # %s", comment);
+
+	return comment;
+}
+
+static void validate_comment_string(const char *comment)
+{
+	size_t leading_blanks;
+
+	if (!comment)
+		return;
+	/*
+	 * The front-end must have massaged the comment string
+	 * properly before calling us.
+	 */
+	if (strchr(comment, '\n'))
+		BUG("multi-line comments are not permitted: '%s'", comment);
+
+	leading_blanks = strspn(comment, " \t");
+	if (!leading_blanks || comment[leading_blanks] != '#')
+		BUG("comment must begin with one or more SP followed by '#': '%s'",
+		    comment);
+}
+
 /*
  * If value==NULL, unset in (remove from) config,
  * if value_pattern!=NULL, disregard key/value pairs where value does not match.
@@ -3211,16 +3267,7 @@ int git_config_set_multivar_in_file_gently(const char *config_filename,
 	size_t contents_sz;
 	struct config_store_data store = CONFIG_STORE_INIT;
 
-	if (comment) {
-		/*
-		 * The front-end must have massaged the comment string
-		 * properly before calling us.
-		 */
-		if (strchr(comment, '\n'))
-			BUG("multi-line comments are not permitted: '%s'", comment);
-		if (comment[0] != '#')
-			BUG("comment should begin with '#': '%s'", comment);
-	}
+	validate_comment_string(comment);
 
 	/* parse-key returns negative; flip the sign to feed exit(3) */
 	ret = 0 - git_config_parse_key(key, &store.key, &store.baselen);
diff --git a/config.h b/config.h
index a85a827169..f4966e3749 100644
--- a/config.h
+++ b/config.h
@@ -338,6 +338,8 @@ void git_config_set_multivar(const char *, const char *, const char *, unsigned)
 int repo_config_set_multivar_gently(struct repository *, const char *, const char *, const char *, unsigned);
 int git_config_set_multivar_in_file_gently(const char *, const char *, const char *, const char *, const char *, unsigned);
 
+const char *git_config_prepare_comment_string(const char *);
+
 /**
  * takes four parameters:
  *
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index d5dfb45877..cc050b3c20 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -74,6 +74,8 @@ cat > expect << EOF
 	penguin = gentoo # Pygoscelis papua
 	disposition = peckish # find fish
 	foo = bar #abc
+	spsp = value # and comment
+	htsp = value	# and comment
 [Sections]
 	WhatEver = Second
 EOF
@@ -82,6 +84,10 @@ test_expect_success 'append comments' '
 	git config --replace-all --comment="Pygoscelis papua" section.penguin gentoo &&
 	git config --comment="find fish" section.disposition peckish &&
 	git config --comment="#abc" section.foo bar &&
+
+	git config --comment="and comment" section.spsp value &&
+	git config --comment="	# and comment" section.htsp value &&
+
 	test_cmp expect .git/config
 '
 
-- 
2.44.0-248-g4f9b731bde


  reply	other threads:[~2024-03-15 22:26 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-04  6:00 [PATCH] Allow git-config to append a comment Ralph Seichter via GitGitGadget
2024-03-06 16:01 ` Junio C Hamano
2024-03-06 17:24   ` Ralph Seichter
2024-03-07 12:12     ` Junio C Hamano
2024-03-07 12:44       ` Ralph Seichter
2024-03-07 13:27         ` Junio C Hamano
2024-03-07 13:53       ` rsbecker
2024-03-07 15:26         ` Ralph Seichter
2024-03-07 15:40           ` rsbecker
2024-03-07 15:57             ` Ralph Seichter
2024-03-07 15:15 ` [PATCH v2] config: add --comment option to add " Ralph Seichter via GitGitGadget
2024-03-11 12:55   ` Junio C Hamano
2024-03-11 16:17     ` Dragan Simic
2024-03-11 16:48       ` rsbecker
2024-03-11 17:00         ` Dragan Simic
2024-03-11 17:52           ` Dragan Simic
2024-03-11 17:29       ` Junio C Hamano
2024-03-11 17:34         ` Dragan Simic
2024-03-11 19:54           ` Junio C Hamano
2024-03-12  2:25             ` Dragan Simic
2024-03-11 18:23       ` Ralph Seichter
2024-03-11 18:50         ` Dragan Simic
2024-03-11 18:57           ` Ralph Seichter
2024-03-11 19:04             ` Dragan Simic
2024-03-11 21:31               ` Junio C Hamano
2024-03-12  2:38                 ` Dragan Simic
2024-03-11 19:17             ` rsbecker
2024-03-12  2:27               ` Dragan Simic
2024-03-11 18:16     ` Ralph Seichter
2024-03-11 18:55       ` Dragan Simic
2024-03-11 19:04         ` Ralph Seichter
2024-03-12  6:19       ` Ralph Seichter
2024-03-12  6:37         ` Chris Torek
2024-03-12  7:28           ` Ralph Seichter
2024-03-12  7:44             ` Junio C Hamano
2024-03-12 21:47   ` [PATCH v3] " Ralph Seichter via GitGitGadget
2024-03-15 22:15     ` Junio C Hamano
2024-03-15 22:26       ` Junio C Hamano [this message]
2024-03-26 22:06         ` [PATCH 3/1] config: allow tweaking whitespace between value and comment Junio C Hamano
2024-03-26 22:48           ` Ralph Seichter
2024-03-26 23:27             ` Junio C Hamano
2024-03-27  0:27               ` Ralph Seichter
2024-03-27  1:23                 ` Dragan Simic
2024-03-27 17:27                 ` Junio C Hamano
2024-03-15 23:10       ` [PATCH v3] config: add --comment option to add a comment Eric Sunshine
2024-03-15 23:41         ` Junio C Hamano
2024-03-15 23:44           ` 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

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqq4jd7p1wf.fsf_-_@gitster.g \
    --to=gitster@pobox.com \
    --cc=chris.torek@gmail.com \
    --cc=dsimic@manjaro.org \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=github@seichter.de \
    --cc=rsbecker@nexbridge.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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.