Subject: [PATCH v4 00/14] builtin/config: introduce subcommands

From: Patrick Steinhardt <[email-masked]>
Date: Fri, 3 May 2024 11:56:40 +0200

Hi,

this is the fourth version of my patch series that introduces
subcommands for git-config(1).

Changes compared to v3:

    - Rebased on top of d4cc1ec35f (Start the 2.46 cycle, 2024-04-30).

    - Implemented support for `git config set --comment`. This switch
      has been added since the last version of this patch series.

Here's hoping that there's a bit more interest in this patch series at
the beginning of the release cycle :)

Patrick

Patrick Steinhardt (14):
  config: clarify memory ownership when preparing comment strings
  builtin/config: move option array around
  builtin/config: move "fixed-value" option to correct group
  builtin/config: use `OPT_CMDMODE()` to specify modes
  builtin/config: pull out function to handle config location
  builtin/config: pull out function to handle `--null`
  builtin/config: introduce "list" subcommand
  builtin/config: introduce "get" subcommand
  builtin/config: introduce "set" subcommand
  builtin/config: introduce "unset" subcommand
  builtin/config: introduce "rename-section" subcommand
  builtin/config: introduce "remove-section" subcommand
  builtin/config: introduce "edit" subcommand
  builtin/config: display subcommand help

 Documentation/git-config.txt | 219 ++++++++-------
 builtin/config.c             | 512 ++++++++++++++++++++++++++++-------
 config.c                     |  16 +-
 config.h                     |   2 +-
 t/t0450/txt-help-mismatches  |   1 -
 t/t1300-config.sh            | 432 +++++++++++++++++------------
 6 files changed, 812 insertions(+), 370 deletions(-)

Range-diff against v3:
 -:  ---------- >  1:  3aa26d5bff config: clarify memory ownership when preparing comment strings
 1:  bfcb50e393 !  2:  8f0804ab48 builtin/config: move option array around
    @@ builtin/config.c: static int option_parse_type(const struct option *opt, const c
     -	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_arg, N_("value"), N_("human-readable comment string (# will be prepended as needed)")),
     -	OPT_END(),
     -};
     -
    @@ builtin/config.c: static char *default_user_config(void)
     +	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_arg, N_("value"), N_("human-readable comment string (# will be prepended as needed)")),
     +	OPT_END(),
     +};
     +
 2:  ff428d8a22 !  3:  ddcd8031d7 builtin/config: move "fixed-value" option to correct group
    @@ builtin/config.c: static struct option builtin_config_options[] = {
      	OPT_BIT(0, "get-color", &actions, N_("find the color configured: slot [default]"), ACTION_GET_COLOR),
      	OPT_BIT(0, "get-colorbool", &actions, N_("find the color setting: slot [stdout-is-tty]"), ACTION_GET_COLORBOOL),
     @@ builtin/config.c: 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_arg, N_("value"), N_("human-readable comment string (# will be prepended as needed)")),
     +	OPT_BOOL(0, "fixed-value", &fixed_value, N_("use string equality when comparing values to 'value-pattern'")),
      	OPT_END(),
      };
 3:  e049c05713 =  4:  1bc3918840 builtin/config: use `OPT_CMDMODE()` to specify modes
 4:  41585803bf !  5:  3754812309 builtin/config: pull out function to handle config location
    @@ builtin/config.c: static char *default_user_config(void)
     -	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_arg, N_("value"), N_("human-readable comment string (# will be prepended as needed)")),
     -	OPT_BOOL(0, "fixed-value", &fixed_value, N_("use string equality when comparing values to 'value-pattern'")),
     -	OPT_END(),
     -};
     -
     -static NORETURN void usage_builtin_config(void)
    --{
    ++static void handle_config_location(const char *prefix)
    + {
     -	usage_with_options(builtin_config_usage, builtin_config_options);
     -}
     -
     -int cmd_config(int argc, const char **argv, const char *prefix)
    -+static void handle_config_location(const char *prefix)
    - {
    +-{
     -	int nongit = !startup_info->have_repository;
    --	char *value = NULL;
    +-	char *value = NULL, *comment = NULL;
     -	int flags = 0;
     -	int ret = 0;
     -	struct key_value_info default_kvi = KVI_INIT;
    @@ builtin/config.c: int cmd_config(int argc, const char **argv, const char *prefix
     +	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_arg, N_("value"), N_("human-readable comment string (# will be prepended as needed)")),
     +	OPT_BOOL(0, "fixed-value", &fixed_value, N_("use string equality when comparing values to 'value-pattern'")),
     +	OPT_END(),
     +};
    @@ builtin/config.c: int cmd_config(int argc, const char **argv, const char *prefix
     +
     +int cmd_config(int argc, const char **argv, const char *prefix)
     +{
    -+	char *value = NULL;
    ++	char *value = NULL, *comment = NULL;
     +	int flags = 0;
     +	int ret = 0;
     +	struct key_value_info default_kvi = KVI_INIT;
 5:  95f661f267 =  6:  cb1714c493 builtin/config: pull out function to handle `--null`
 6:  b50f32d074 !  7:  b3f3c3ba6a builtin/config: introduce "list" subcommand
    @@ Documentation/git-config.txt: git-config - Get and set repository or global opti
      --------
      [verse]
     +'git config list' [<file-option>] [<display-option>] [--includes]
    - 'git config' [<file-option>] [--type=<type>] [--fixed-value] [--show-origin] [--show-scope] [-z|--null] <name> [<value> [<value-pattern>]]
    - 'git config' [<file-option>] [--type=<type>] --add <name> <value>
    - 'git config' [<file-option>] [--type=<type>] [--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>]
     @@ Documentation/git-config.txt: SYNOPSIS
      'git config' [<file-option>] [--fixed-value] --unset-all <name> [<value-pattern>]
      'git config' [<file-option>] --rename-section <old-name> <new-name>
    @@ builtin/config.c: 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_arg, N_("value"), N_("human-readable comment string (# will be prepended as needed)")),
      	OPT_BOOL(0, "fixed-value", &fixed_value, N_("use string equality when comparing values to 'value-pattern'")),
     +	OPT_BOOL(0, "includes", &respect_includes_opt, N_("respect include directives on lookup")),
      	OPT_END(),
    @@ builtin/config.c: static NORETURN void usage_builtin_config(void)
     +
      int cmd_config(int argc, const char **argv, const char *prefix)
      {
    - 	char *value = NULL;
    + 	char *value = NULL, *comment = NULL;
     @@ builtin/config.c: int cmd_config(int argc, const char **argv, const char *prefix)
      
      	given_config_source.file = xstrdup_or_null(getenv(CONFIG_ENVIRONMENT));
    @@ t/t1300-config.sh: export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
     +	BUG "unknown mode $mode";;
     +esac
     +
    - test_expect_success 'clear default config' '
    - 	rm -f .git/config
    - '
    + test_expect_success 'setup whitespace config' '
    + 	sed -e "s/^|//" \
    + 	    -e "s/[$]$//" \
     @@ t/t1300-config.sh: version.1.2.3eX.alpha=beta
      EOF
      
    @@ t/t1300-config.sh: Qsection.sub=section.val4
      	nul_to_q <result.raw >result &&
      	echo >>result &&
      	test_cmp expect result
    -@@ t/t1300-config.sh: test_expect_success 'inner whitespace kept verbatim' '
    +@@ t/t1300-config.sh: test_expect_success 'inner whitespace kept verbatim, horizontal tabs and spaces'
      '
      
      test_expect_success SYMLINKS 'symlinked configuration' '
 7:  eee1fae50c !  8:  0e6da909ac builtin/config: introduce "get" subcommand
    @@ Documentation/git-config.txt: SYNOPSIS
      [verse]
      'git config list' [<file-option>] [<display-option>] [--includes]
     +'git config get' [<file-option>] [<display-option>] [--includes] [--all] [--regexp=<regexp>] [--value=<value>] [--fixed-value] [--default=<default>] <name>
    - 'git config' [<file-option>] [--type=<type>] [--fixed-value] [--show-origin] [--show-scope] [-z|--null] <name> [<value> [<value-pattern>]]
    - 'git config' [<file-option>] [--type=<type>] --add <name> <value>
    - 'git config' [<file-option>] [--type=<type>] [--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>]
    @@ Documentation/git-config.txt: COMMANDS
      OPTIONS
      -------
     @@ Documentation/git-config.txt: OPTIONS
    - 	values.  This is the same as providing '^$' as the `value-pattern`
    - 	in `--replace-all`.
    + 	not contain linefeed characters (no multi-line comments are
    + 	permitted).
      
     ---get::
     -	Get the value for a given key (optionally filtered by a regex
    @@ Documentation/git-config.txt: OPTIONS
     ---get-all::
     -	Like get, but returns all values for a multi-valued key.
     +--all::
    -+	With `get`, Return all values for a multi-valued key.
    ++	With `get`, return all values for a multi-valued key.
      
     ---get-regexp::
     -	Like --get-all, but interprets the name as a regular expression and
    @@ t/t1300-config.sh: test_expect_success '--null --list' '
      	nul_to_q <result.raw >result &&
      	echo >>result &&
      	test_cmp expect result
    +@@ t/t1300-config.sh: test_expect_success '--null --get-regexp' '
    + test_expect_success 'inner whitespace kept verbatim, spaces only' '
    + 	echo "foo   bar" >expect &&
    + 	git config section.val "foo   bar" &&
    +-	git config --get section.val >actual &&
    ++	git config ${mode_get} section.val >actual &&
    + 	test_cmp expect actual
    + '
    + 
    + test_expect_success 'inner whitespace kept verbatim, horizontal tabs only' '
    + 	echo "fooQQbar" | q_to_tab >expect &&
    + 	git config section.val "$(cat expect)" &&
    +-	git config --get section.val >actual &&
    ++	git config ${mode_get} section.val >actual &&
    + 	test_cmp expect actual
    + '
    + 
    + test_expect_success 'inner whitespace kept verbatim, horizontal tabs and spaces' '
    + 	echo "foo Q  bar" | q_to_tab >expect &&
    + 	git config section.val "$(cat expect)" &&
    +-	git config --get section.val >actual &&
    ++	git config ${mode_get} section.val >actual &&
    + 	test_cmp expect actual
    + '
    + 
     @@ t/t1300-config.sh: test_expect_success 'git -c can represent empty string' '
      '
      
 8:  e2815affab !  9:  8a623a31b9 builtin/config: introduce "set" subcommand
    @@ Documentation/git-config.txt: SYNOPSIS
      [verse]
      'git config list' [<file-option>] [<display-option>] [--includes]
      'git config get' [<file-option>] [<display-option>] [--includes] [--all] [--regexp=<regexp>] [--value=<value>] [--fixed-value] [--default=<default>] <name>
    --'git config' [<file-option>] [--type=<type>] [--fixed-value] [--show-origin] [--show-scope] [-z|--null] <name> [<value> [<value-pattern>]]
    --'git config' [<file-option>] [--type=<type>] --add <name> <value>
    --'git config' [<file-option>] [--type=<type>] [--fixed-value] --replace-all <name> <value> [<value-pattern>]
    -+'git config set' [<file-option>] [--type=<type>] [--all] [--value=<value>] [--fixed-value] <name> <value>
    +-'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 set' [<file-option>] [--type=<type>] [--comment=<message>] [--all] [--value=<value>] [--fixed-value] <name> <value>
      'git config' [<file-option>] [--fixed-value] --unset <name> [<value-pattern>]
      'git config' [<file-option>] [--fixed-value] --unset-all <name> [<value-pattern>]
      'git config' [<file-option>] --rename-section <old-name> <new-name>
    @@ Documentation/git-config.txt: OPTIONS
     -	in `--replace-all`.
     +	values. This is the same as providing '--value=^$' in `set`.
      
    - --all::
    - 	With `get`, Return all values for a multi-valued key.
    + --comment <message>::
    + 	Append a comment at the end of new or modified lines.
     @@ Documentation/git-config.txt: recommended to migrate to the new syntax.
      'git config <name>'::
      	Replaced by `git config get <name>`.
    @@ builtin/config.c: static const char *const builtin_config_get_usage[] = {
      };
      
     +static const char *const builtin_config_set_usage[] = {
    -+	N_("git config set [<file-option>] [--type=<type>] [--all] [--value=<value>] [--fixed-value] <name> <value>"),
    ++	N_("git config set [<file-option>] [--type=<type>] [--comment=<message>] [--all] [--value=<value>] [--fixed-value] <name> <value>"),
     +	NULL
     +};
     +
    @@ builtin/config.c: static int cmd_config_get(int argc, const char **argv, const c
      
     +static int cmd_config_set(int argc, const char **argv, const char *prefix)
     +{
    -+	const char *value_pattern = NULL;
    ++	const char *value_pattern = NULL, *comment_arg = NULL;
    ++	char *comment = NULL;
     +	int flags = 0, append = 0;
     +	struct option opts[] = {
     +		CONFIG_LOCATION_OPTIONS,
    @@ builtin/config.c: static int cmd_config_get(int argc, const char **argv, const c
     +		OPT_STRING(0, "value", &value_pattern, N_("pattern"), N_("show config with values matching the pattern")),
     +		OPT_BIT(0, "fixed-value", &flags, N_("use string equality when comparing values to value pattern"), CONFIG_FLAGS_FIXED_VALUE),
     +		OPT_GROUP(N_("Other")),
    ++		OPT_STRING(0, "comment", &comment_arg, N_("value"), N_("human-readable comment string (# will be prepended as needed)")),
     +		OPT_BOOL(0, "append", &append, N_("add a new line without altering any existing values")),
     +		OPT_END(),
     +	};
    @@ builtin/config.c: static int cmd_config_get(int argc, const char **argv, const c
     +	if (append)
     +		value_pattern = CONFIG_REGEX_NONE;
     +
    ++	comment = git_config_prepare_comment_string(comment_arg);
    ++
     +	handle_config_location(prefix);
     +
     +	value = normalize_value(argv[0], argv[1], &default_kvi);
    @@ builtin/config.c: static int cmd_config_get(int argc, const char **argv, const c
     +	if ((flags & CONFIG_FLAGS_MULTI_REPLACE) || value_pattern) {
     +		ret = git_config_set_multivar_in_file_gently(given_config_source.file,
     +							     argv[0], value, value_pattern,
    -+							     flags);
    ++							     comment, flags);
     +	} else {
    -+		ret = git_config_set_in_file_gently(given_config_source.file, argv[0], value);
    ++		ret = git_config_set_in_file_gently(given_config_source.file,
    ++						    argv[0], comment, value);
     +		if (ret == CONFIG_NOTHING_SET)
     +			error(_("cannot overwrite multiple values with a single value\n"
     +			"       Use a regexp, --add or --replace-all to change %s."), argv[0]);
     +	}
     +
    ++	free(comment);
     +	free(value);
     +	return ret;
     +}
    @@ t/t1300-config.sh: cat > expect << EOF
      	test_cmp expect .git/config
      '
      
    +@@ t/t1300-config.sh: 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 ${mode_set} --comment="find fish" section.disposition peckish &&
    ++	git config ${mode_set} --comment="#abc" section.foo bar &&
    + 
    + 	git config --comment="and comment" section.spsp value &&
    + 	git config --comment="	# and comment" section.htsp value &&
    +@@ t/t1300-config.sh: test_expect_success 'append comments' '
    + '
    + 
    + test_expect_success 'Prohibited LF in comment' '
    +-	test_must_fail git config --comment="a${LF}b" section.k v
    ++	test_must_fail git config ${mode_set} --comment="a${LF}b" section.k v
    + '
    + 
    + test_expect_success 'non-match result' 'test_cmp expect .git/config'
     @@ t/t1300-config.sh: test_expect_success 'multiple unset is correct' '
      cp .git/config2 .git/config
      
    @@ t/t1300-config.sh: test_expect_success 'key with newline' '
      
      cat > .git/config <<\EOF
     @@ t/t1300-config.sh: test_expect_success '--null --get-regexp' '
    + 
    + test_expect_success 'inner whitespace kept verbatim, spaces only' '
    + 	echo "foo   bar" >expect &&
    +-	git config section.val "foo   bar" &&
    ++	git config ${mode_set} section.val "foo   bar" &&
    + 	git config ${mode_get} section.val >actual &&
    + 	test_cmp expect actual
      '
      
    - test_expect_success 'inner whitespace kept verbatim' '
    --	git config section.val "foo 	  bar" &&
    -+	git config ${mode_set} section.val "foo 	  bar" &&
    - 	test_cmp_config "foo 	  bar" section.val
    + test_expect_success 'inner whitespace kept verbatim, horizontal tabs only' '
    + 	echo "fooQQbar" | q_to_tab >expect &&
    +-	git config section.val "$(cat expect)" &&
    ++	git config ${mode_set} section.val "$(cat expect)" &&
    + 	git config ${mode_get} section.val >actual &&
    + 	test_cmp expect actual
      '
      
    + test_expect_success 'inner whitespace kept verbatim, horizontal tabs and spaces' '
    + 	echo "foo Q  bar" | q_to_tab >expect &&
    +-	git config section.val "$(cat expect)" &&
    ++	git config ${mode_set} section.val "$(cat expect)" &&
    + 	git config ${mode_get} section.val >actual &&
    + 	test_cmp expect actual
    + '
     @@ t/t1300-config.sh: test_expect_success 'check split_cmdline return' '
      	git init repo &&
      	(
 9:  90f055ae1d ! 10:  e25e5b69cd builtin/config: introduce "unset" subcommand
    @@ Commit message
     
      ## Documentation/git-config.txt ##
     @@ Documentation/git-config.txt: SYNOPSIS
    + [verse]
      'git config list' [<file-option>] [<display-option>] [--includes]
      'git config get' [<file-option>] [<display-option>] [--includes] [--all] [--regexp=<regexp>] [--value=<value>] [--fixed-value] [--default=<default>] <name>
    - 'git config set' [<file-option>] [--type=<type>] [--all] [--value=<value>] [--fixed-value] <name> <value>
    +-'git config set' [<file-option>] [--type=<type>] [--comment=<message>] [--all] [--value=<value>] [--fixed-value] <name> <value>
     -'git config' [<file-option>] [--fixed-value] --unset <name> [<value-pattern>]
     -'git config' [<file-option>] [--fixed-value] --unset-all <name> [<value-pattern>]
    ++'git config set' [<file-option>] [--type=<type>] [--all] [--value=<value>] [--fixed-value] <name> <value>
     +'git config unset' [<file-option>] [--all] [--value=<value>] [--fixed-value] <name> <value>
      'git config' [<file-option>] --rename-section <old-name> <new-name>
      'git config' [<file-option>] --remove-section <name>
    @@ builtin/config.c: static int cmd_config_set(int argc, const char **argv, const c
     +	if ((flags & CONFIG_FLAGS_MULTI_REPLACE) || value_pattern)
     +		return git_config_set_multivar_in_file_gently(given_config_source.file,
     +							      argv[0], NULL, value_pattern,
    -+							      flags);
    ++							      NULL, flags);
     +	else
    -+		return git_config_set_in_file_gently(given_config_source.file, argv[0], NULL);
    ++		return git_config_set_in_file_gently(given_config_source.file, argv[0],
    ++						     NULL, NULL);
     +}
     +
      static struct option builtin_subcommand_options[] = {
10:  3e360b1f47 ! 11:  f24008d356 builtin/config: introduce "rename-section" subcommand
    @@ builtin/config.c: static const char *const builtin_config_unset_usage[] = {
      static regex_t *key_regexp;
      static const char *value_pattern;
     @@ builtin/config.c: static int cmd_config_unset(int argc, const char **argv, const char *prefix)
    - 		return git_config_set_in_file_gently(given_config_source.file, argv[0], NULL);
    + 						     NULL, NULL);
      }
      
     +static int cmd_config_rename_section(int argc, const char **argv, const char *prefix)
11:  d610b5fda1 = 12:  fc2ddd3201 builtin/config: introduce "remove-section" subcommand
12:  4a6512c88a = 13:  4c2d817eff builtin/config: introduce "edit" subcommand
13:  657d1355b5 = 14:  4c351b12b8 builtin/config: display subcommand help
-- 
2.45.0