git.vger.kernel.org archive mirror
 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 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 NNTP newsgroup(s).