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>

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index e608d5ffef236aeb9cb958477b9e1987db85887d..af374ee2e0fce42b01bcac880bbf0d3b7b62b139 100644 (file)
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -9,9 +9,9 @@ git-config - Get and set repository or global options
 SYNOPSIS
 --------
 [verse]
-'git config' [<file-option>] [--type=<type>] [--comment=<value>] [--fixed-value] [--show-origin] [--show-scope] [-z|--null] <name> [<value> [<value-pattern>]]
-'git config' [<file-option>] [--type=<type>] [--comment=<value>] --add <name> <value>
-'git config' [<file-option>] [--type=<type>] [--comment=<value>] [--fixed-value] --replace-all <name> <value> [<value-pattern>]
+'git config' [<file-option>] [--type=<type>] [--comment=<message>] [--fixed-value] [--show-origin] [--show-scope] [-z|--null] <name> [<value> [<value-pattern>]]
+'git config' [<file-option>] [--type=<type>] [--comment=<message>] --add <name> <value>
+'git config' [<file-option>] [--type=<type>] [--comment=<message>] [--fixed-value] --replace-all <name> <value> [<value-pattern>]
 'git config' [<file-option>] [--type=<type>] [--show-origin] [--show-scope] [-z|--null] [--fixed-value] --get <name> [<value-pattern>]
 'git config' [<file-option>] [--type=<type>] [--show-origin] [--show-scope] [-z|--null] [--fixed-value] --get-all <name> [<value-pattern>]
 '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
        values.  This is the same as providing '^$' as the `value-pattern`
        in `--replace-all`.
 
---comment <value>::
-       Append a comment to new or modified lines. A '#' character will be
-       unconditionally prepended to the value. The value must not contain
-       linefeed characters (no multi-line comments are permitted).
+--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).
 
 --get::
        Get the value for a given key (optionally filtered by a regex

diff --git a/builtin/config.c b/builtin/config.c
index c54e9941a5ff7c4df4fc60c3ac2754f616e241ce..e859a659f44eae50ae6433957d5046f67bd9c5de 100644 (file)
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -174,7 +174,7 @@ static struct option builtin_config_options[] = {
        OPT_BOOL(0, "show-origin", &show_origin, N_("show origin of config (file, standard input, blob, command line)")),
        OPT_BOOL(0, "show-scope", &show_scope, N_("show scope of config (worktree, local, global, system, command)")),
        OPT_STRING(0, "default", &default_value, N_("value"), N_("with --get, use default value when missing entry")),
-       OPT_STRING(0, "comment", &comment, N_("value"), N_("human-readable comment string (# will be prepended automatically)")),
+       OPT_STRING(0, "comment", &comment, N_("value"), N_("human-readable comment string (# will be prepended as needed)")),
        OPT_END(),
 };
 
@@ -800,9 +800,9 @@ int cmd_config(int argc, const char **argv, const char *prefix)
        }
 
        if (comment &&
-               !(actions & (ACTION_ADD|ACTION_SET|ACTION_SET_ALL|ACTION_REPLACE_ALL))) {
-                       error(_("--comment is only applicable to add/set/replace operations"));
-                       usage_builtin_config();
+           !(actions & (ACTION_ADD|ACTION_SET|ACTION_SET_ALL|ACTION_REPLACE_ALL))) {
+               error(_("--comment is only applicable to add/set/replace operations"));
+               usage_builtin_config();
        }
 
        /* check usage of --fixed-value */
@@ -841,6 +841,13 @@ 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);
+       }
+
        if (actions & PAGING_ACTIONS)
                setup_auto_pager("config", 1);
 

diff --git a/config.c b/config.c
index 2f3f6d123c64b48d32fdc3eea83ad72a1fd1a85b..15019cb9a5ccf5d221a34f5c87a34e09f4b5d6a4 100644 (file)
--- a/config.c
+++ b/config.c
@@ -3043,12 +3043,9 @@ static ssize_t write_pair(int fd, const char *key, const char *value,
                        break;
                }
 
-       if (comment) {
-               if (strchr(comment, '\n'))
-                       die(_("multi-line comments are not permitted: '%s'"), comment);
-               else
-                       strbuf_addf(&sb, "%s #%s\n", quote, comment);
-       } else
+       if (comment)
+               strbuf_addf(&sb, "%s %s\n", quote, comment);
+       else
                strbuf_addf(&sb, "%s\n", quote);
 
        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,
        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);
+       }
+
        /* parse-key returns negative; flip the sign to feed exit(3) */
        ret = 0 - git_config_parse_key(key, &store.key, &store.baselen);
        if (ret)

diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index ac7b02e6b07a6753bb7234bbd1158cb862231676..d5dfb45877f52961b572f4f385ab0f69347464dc 100755 (executable)
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -71,16 +71,17 @@ cat > expect << EOF
 [section]
        Movie = BadPhysics
        UPPERCASE = true
-       penguin = gentoo #Pygoscelis papua
-       disposition = peckish #find fish
-       foo = bar ## abc
+       penguin = gentoo # Pygoscelis papua
+       disposition = peckish # find fish
+       foo = bar #abc
 [Sections]
        WhatEver = Second
 EOF
+
 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="#abc" section.foo bar &&
        test_cmp expect .git/config
 '