config: fix --comment formatting

When git adds comments itself (like "rebase -i" todo list and "commit -e" log message editor), it always gives a comment introducer "#" followed by a Space before the message, except for the recently introduced "git config --comment", where the users are forced to say " this is my comment" if they want to add their comment in this usual format; otherwise their comment string will end up without a space after the "#". Make it more ergonomic, while keeping it possible to also use this unusual style, by massaging the comment string at the UI layer with a set of simple rules: * If the given comment string begins with '#', it is passed intact. * Otherwise, "# " is prefixed. * A string with LF in it cannot be used as a comment string. Right now there is only one "front-end" that accepts end-user comment string and calls the underlying machinery to add or modify configuration file with comments, but to make sure that the future callers perform similar massaging as they see fit, add a sanity check logic in git_config_set_multivar_in_file_gently(), which is the single choke point in the codepaths that consumes the comment string. Signed-off-by: Junio C Hamano <gitster@pobox.com>

Junio C Hamano committed Mar 15, 2024 at 12:43 UTC fbad334db9cff30cdf1fe3d498dec737bfae38df
4 files changed +38 -21
Documentation/git-config.txt
+8 -7
@@ -9,9 +9,9 @@ git-config - Get and set repository or global options
9 SYNOPSIS
10 --------
11 [verse]
12 -'git config' [<file-option>] [--type=<type>] [--comment=<value>] [--fixed-value] [--show-origin] [--show-scope] [-z|--null] <name> [<value> [<value-pattern>]]
13 -'git config' [<file-option>] [--type=<type>] [--comment=<value>] --add <name> <value>
14 -'git config' [<file-option>] [--type=<type>] [--comment=<value>] [--fixed-value] --replace-all <name> <value> [<value-pattern>]
12 +'git config' [<file-option>] [--type=<type>] [--comment=<message>] [--fixed-value] [--show-origin] [--show-scope] [-z|--null] <name> [<value> [<value-pattern>]]
13 +'git config' [<file-option>] [--type=<type>] [--comment=<message>] --add <name> <value>
14 +'git config' [<file-option>] [--type=<type>] [--comment=<message>] [--fixed-value] --replace-all <name> <value> [<value-pattern>]
15 'git config' [<file-option>] [--type=<type>] [--show-origin] [--show-scope] [-z|--null] [--fixed-value] --get <name> [<value-pattern>]
16 'git config' [<file-option>] [--type=<type>] [--show-origin] [--show-scope] [-z|--null] [--fixed-value] --get-all <name> [<value-pattern>]
17 'git config' [<file-option>] [--type=<type>] [--show-origin] [--show-scope] [-z|--null] [--fixed-value] [--name-only] --get-regexp <name-regex> [<value-pattern>]
@@ -87,10 +87,11 @@ OPTIONS
87 values. This is the same as providing '^$' as the `value-pattern`
88 in `--replace-all`.
89
90 ---comment <value>::
91 - Append a comment to new or modified lines. A '#' character will be
92 - unconditionally prepended to the value. The value must not contain
93 - linefeed characters (no multi-line comments are permitted).
90 +--comment <message>::
91 + Append a comment at the end of new or modified lines.
92 + Unless _<message>_ begins with "#", a string "# " (hash
93 + followed by a space) is prepended to it. The _<message>_ must not
94 + contain linefeed characters (no multi-line comments are permitted).
95
96 --get::
97 Get the value for a given key (optionally filtered by a regex
builtin/config.c
+11 -4
@@ -174,7 +174,7 @@ static struct option builtin_config_options[] = {
174 OPT_BOOL(0, "show-origin", &show_origin, N_("show origin of config (file, standard input, blob, command line)")),
175 OPT_BOOL(0, "show-scope", &show_scope, N_("show scope of config (worktree, local, global, system, command)")),
176 OPT_STRING(0, "default", &default_value, N_("value"), N_("with --get, use default value when missing entry")),
177 - OPT_STRING(0, "comment", &comment, N_("value"), N_("human-readable comment string (# will be prepended automatically)")),
177 + OPT_STRING(0, "comment", &comment, N_("value"), N_("human-readable comment string (# will be prepended as needed)")),
178 OPT_END(),
179 };
180
@@ -800,9 +800,9 @@ int cmd_config(int argc, const char **argv, const char *prefix)
800 }
801
802 if (comment &&
803 - !(actions & (ACTION_ADD|ACTION_SET|ACTION_SET_ALL|ACTION_REPLACE_ALL))) {
804 - error(_("--comment is only applicable to add/set/replace operations"));
805 - usage_builtin_config();
803 + !(actions & (ACTION_ADD|ACTION_SET|ACTION_SET_ALL|ACTION_REPLACE_ALL))) {
804 + error(_("--comment is only applicable to add/set/replace operations"));
805 + usage_builtin_config();
806 }
807
808 /* check usage of --fixed-value */
@@ -841,6 +841,13 @@ int cmd_config(int argc, const char **argv, const char *prefix)
841 flags |= CONFIG_FLAGS_FIXED_VALUE;
842 }
843
844 + if (comment) {
845 + if (strchr(comment, '\n'))
846 + die(_("no multi-line comment allowed: '%s'"), comment);
847 + if (comment[0] != '#')
848 + comment = xstrfmt("# %s", comment);
849 + }
850 +
851 if (actions & PAGING_ACTIONS)
852 setup_auto_pager("config", 1);
853
config.c
+14 -6
@@ -3043,12 +3043,9 @@ static ssize_t write_pair(int fd, const char *key, const char *value,
3043 break;
3044 }
3045
3046 - if (comment) {
3047 - if (strchr(comment, '\n'))
3048 - die(_("multi-line comments are not permitted: '%s'"), comment);
3049 - else
3050 - strbuf_addf(&sb, "%s #%s\n", quote, comment);
3051 - } else
3046 + if (comment)
3047 + strbuf_addf(&sb, "%s %s\n", quote, comment);
3048 + else
3049 strbuf_addf(&sb, "%s\n", quote);
3050
3051 ret = write_in_full(fd, sb.buf, sb.len);
@@ -3214,6 +3211,17 @@ int git_config_set_multivar_in_file_gently(const char *config_filename,
3211 size_t contents_sz;
3212 struct config_store_data store = CONFIG_STORE_INIT;
3213
3214 + if (comment) {
3215 + /*
3216 + * The front-end must have massaged the comment string
3217 + * properly before calling us.
3218 + */
3219 + if (strchr(comment, '\n'))
3220 + BUG("multi-line comments are not permitted: '%s'", comment);
3221 + if (comment[0] != '#')
3222 + BUG("comment should begin with '#': '%s'", comment);
3223 + }
3224 +
3225 /* parse-key returns negative; flip the sign to feed exit(3) */
3226 ret = 0 - git_config_parse_key(key, &store.key, &store.baselen);
3227 if (ret)
t/t1300-config.sh
+5 -4
@@ -71,16 +71,17 @@ cat > expect << EOF
71 [section]
72 Movie = BadPhysics
73 UPPERCASE = true
74 - penguin = gentoo #Pygoscelis papua
75 - disposition = peckish #find fish
76 - foo = bar ## abc
74 + penguin = gentoo # Pygoscelis papua
75 + disposition = peckish # find fish
76 + foo = bar #abc
77 [Sections]
78 WhatEver = Second
79 EOF
80 +
81 test_expect_success 'append comments' '
82 git config --replace-all --comment="Pygoscelis papua" section.penguin gentoo &&
83 git config --comment="find fish" section.disposition peckish &&
83 - git config --comment="# abc" section.foo bar &&
84 + git config --comment="#abc" section.foo bar &&
85 test_cmp expect .git/config
86 '
87