[PATCH 0/1] Remove value from positional args in config unset usage

[PATCH 0/1] Remove value from positional args in config unset usage

[Josh Heinrichs]

I tripped over this while attempting to use `git config unset --fixed-value`.
The value is supposed to be provided via `--value`, but the usage makes it
seem like it can be provided as a positional argument.

While I was looking at the translations I noticed that po/bg.po was missing
`<name>` as a positional argument. From my reading of the docs it sounds like
translation changes are made independently, so I have not changed them in this
series.

Josh Heinrichs (1):
  git-config.1: remove value from positional args in unset usage

 Documentation/git-config.txt | 2 +-
 builtin/config.c             | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

-- 
2.25.1

[PATCH 1/1] git-config.1: remove value from positional args in unset usage

[Josh Heinrichs]

A single positional argument appears to be enforcecd at
builtin/config.c:993. Values need to be provided via --value.
---
 Documentation/git-config.txt | 2 +-
 builtin/config.c             | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 7f81fbbea8..3e420177c1 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -12,7 +12,7 @@ SYNOPSIS
 'git config list' [<file-option>] [<display-option>] [--includes]
 'git config get' [<file-option>] [<display-option>] [--includes] [--all] [--regexp] [--value=<value>] [--fixed-value] [--default=<default>] <name>
 '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 unset' [<file-option>] [--all] [--value=<value>] [--fixed-value] <name>
 'git config rename-section' [<file-option>] <old-name> <new-name>
 'git config remove-section' [<file-option>] <name>
 'git config edit' [<file-option>]
diff --git a/builtin/config.c b/builtin/config.c
index 97e4d5f57c..d60e106bd6 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -19,7 +19,7 @@ static const char *const builtin_config_usage[] = {
 	N_("git config list [<file-option>] [<display-option>] [--includes]"),
 	N_("git config get [<file-option>] [<display-option>] [--includes] [--all] [--regexp] [--value=<value>] [--fixed-value] [--default=<default>] <name>"),
 	N_("git config set [<file-option>] [--type=<type>] [--all] [--value=<value>] [--fixed-value] <name> <value>"),
-	N_("git config unset [<file-option>] [--all] [--value=<value>] [--fixed-value] <name> <value>"),
+	N_("git config unset [<file-option>] [--all] [--value=<value>] [--fixed-value] <name>"),
 	N_("git config rename-section [<file-option>] <old-name> <new-name>"),
 	N_("git config remove-section [<file-option>] <name>"),
 	N_("git config edit [<file-option>]"),
@@ -43,7 +43,7 @@ static const char *const builtin_config_set_usage[] = {
 };
 
 static const char *const builtin_config_unset_usage[] = {
-	N_("git config unset [<file-option>] [--all] [--value=<value>] [--fixed-value] <name> <value>"),
+	N_("git config unset [<file-option>] [--all] [--value=<value>] [--fixed-value] <name>"),
 	NULL
 };
 
-- 
2.25.1

Re: [PATCH 1/1] git-config.1: remove value from positional args in unset usage

[Patrick Steinhardt]

On Mon, Oct 07, 2024 at 10:07:49PM -0600, Josh Heinrichs wrote:
> A single positional argument appears to be enforcecd at
> builtin/config.c:993. Values need to be provided via --value.

Good catch!

Two comments regarding the commit message:

  - You are missing the Signed-off-by tag.

  - Our commit messages are typically structured such that they first
    give an observation of the issue and why it is an issue, followed by
    a proposed solution and imperative instructions for the code to
    become so.

So something like the following:

  The synopsis for the `git config unset` command mentions two
  positional arguments `<name>` and `<value>`. While the first
  positional argument is indeed correct, the second one is not. Instead,
  users are expected to pass in the value via the `--value=<value>`
  option.

  Remove the positional argument. The `--value=<value>` option is
  already documented correctly, so this is all we need to do to fix up
  the documentation.

Other than that the change looks good to me, thanks!

Patrick

[PATCH v2 0/1] Remove value from position args in config unset usage

[Josh Heinrichs]

Hello!

This version addresses the issues with my commit message that Patrick flagged
in v1.

Josh

Josh Heinrichs (1):
  git-config.1: remove value from positional args in unset usage

 Documentation/git-config.txt | 2 +-
 builtin/config.c             | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

Range-diff against v1:
1:  4bb9c65fb9 ! 1:  e3b5f3c8b2 git-config.1: remove value from positional args in unset usage
    @@ Metadata
      ## Commit message ##
         git-config.1: remove value from positional args in unset usage
     
    -    A single positional argument appears to be enforcecd at
    -    builtin/config.c:993. Values need to be provided via --value.
    +    The synopsis for `git config unset` mentions two positional arguments:
    +    `<name>` and `<value>`. While the first argument is correct, the second
    +    is not. Users are expected to provide the value via `--value=<value>`.
    +
    +    Remove the positional argument. The `--value=<value>` option is already
    +    documented correctly, so this is all we need to do to fix the
    +    documentation.
    +
    +    Signed-off-by: Josh Heinrichs <joshiheinrichs@gmail.com>
     
      ## Documentation/git-config.txt ##
     @@ Documentation/git-config.txt: SYNOPSIS
-- 
2.25.1

[PATCH v2 1/1] git-config.1: remove value from positional args in unset usage

[Josh Heinrichs]

The synopsis for `git config unset` mentions two positional arguments:
`<name>` and `<value>`. While the first argument is correct, the second
is not. Users are expected to provide the value via `--value=<value>`.

Remove the positional argument. The `--value=<value>` option is already
documented correctly, so this is all we need to do to fix the
documentation.

Signed-off-by: Josh Heinrichs <joshiheinrichs@gmail.com>
---
 Documentation/git-config.txt | 2 +-
 builtin/config.c             | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 7f81fbbea8..3e420177c1 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -12,7 +12,7 @@ SYNOPSIS
 'git config list' [<file-option>] [<display-option>] [--includes]
 'git config get' [<file-option>] [<display-option>] [--includes] [--all] [--regexp] [--value=<value>] [--fixed-value] [--default=<default>] <name>
 '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 unset' [<file-option>] [--all] [--value=<value>] [--fixed-value] <name>
 'git config rename-section' [<file-option>] <old-name> <new-name>
 'git config remove-section' [<file-option>] <name>
 'git config edit' [<file-option>]
diff --git a/builtin/config.c b/builtin/config.c
index 97e4d5f57c..d60e106bd6 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -19,7 +19,7 @@ static const char *const builtin_config_usage[] = {
 	N_("git config list [<file-option>] [<display-option>] [--includes]"),
 	N_("git config get [<file-option>] [<display-option>] [--includes] [--all] [--regexp] [--value=<value>] [--fixed-value] [--default=<default>] <name>"),
 	N_("git config set [<file-option>] [--type=<type>] [--all] [--value=<value>] [--fixed-value] <name> <value>"),
-	N_("git config unset [<file-option>] [--all] [--value=<value>] [--fixed-value] <name> <value>"),
+	N_("git config unset [<file-option>] [--all] [--value=<value>] [--fixed-value] <name>"),
 	N_("git config rename-section [<file-option>] <old-name> <new-name>"),
 	N_("git config remove-section [<file-option>] <name>"),
 	N_("git config edit [<file-option>]"),
@@ -43,7 +43,7 @@ static const char *const builtin_config_set_usage[] = {
 };
 
 static const char *const builtin_config_unset_usage[] = {
-	N_("git config unset [<file-option>] [--all] [--value=<value>] [--fixed-value] <name> <value>"),
+	N_("git config unset [<file-option>] [--all] [--value=<value>] [--fixed-value] <name>"),
 	NULL
 };
 
-- 
2.25.1

Re: [PATCH v2 1/1] git-config.1: remove value from positional args in unset usage

[Junio C Hamano]

Josh Heinrichs <joshiheinrichs@gmail.com> writes:

> The synopsis for `git config unset` mentions two positional arguments:
> `<name>` and `<value>`. While the first argument is correct, the second
> is not. Users are expected to provide the value via `--value=<value>`.
>
> Remove the positional argument. The `--value=<value>` option is already
> documented correctly, so this is all we need to do to fix the
> documentation.
>
> Signed-off-by: Josh Heinrichs <joshiheinrichs@gmail.com>
> ---
>  Documentation/git-config.txt | 2 +-
>  builtin/config.c             | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)

Patrick, do you mind to forge your Reviewed-by for this one?

Will queue.  Thanks, both.

> diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
> index 7f81fbbea8..3e420177c1 100644
> --- a/Documentation/git-config.txt
> +++ b/Documentation/git-config.txt
> @@ -12,7 +12,7 @@ SYNOPSIS
>  'git config list' [<file-option>] [<display-option>] [--includes]
>  'git config get' [<file-option>] [<display-option>] [--includes] [--all] [--regexp] [--value=<value>] [--fixed-value] [--default=<default>] <name>
>  '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 unset' [<file-option>] [--all] [--value=<value>] [--fixed-value] <name>
>  'git config rename-section' [<file-option>] <old-name> <new-name>
>  'git config remove-section' [<file-option>] <name>
>  'git config edit' [<file-option>]
> diff --git a/builtin/config.c b/builtin/config.c
> index 97e4d5f57c..d60e106bd6 100644
> --- a/builtin/config.c
> +++ b/builtin/config.c
> @@ -19,7 +19,7 @@ static const char *const builtin_config_usage[] = {
>  	N_("git config list [<file-option>] [<display-option>] [--includes]"),
>  	N_("git config get [<file-option>] [<display-option>] [--includes] [--all] [--regexp] [--value=<value>] [--fixed-value] [--default=<default>] <name>"),
>  	N_("git config set [<file-option>] [--type=<type>] [--all] [--value=<value>] [--fixed-value] <name> <value>"),
> -	N_("git config unset [<file-option>] [--all] [--value=<value>] [--fixed-value] <name> <value>"),
> +	N_("git config unset [<file-option>] [--all] [--value=<value>] [--fixed-value] <name>"),
>  	N_("git config rename-section [<file-option>] <old-name> <new-name>"),
>  	N_("git config remove-section [<file-option>] <name>"),
>  	N_("git config edit [<file-option>]"),
> @@ -43,7 +43,7 @@ static const char *const builtin_config_set_usage[] = {
>  };
>  
>  static const char *const builtin_config_unset_usage[] = {
> -	N_("git config unset [<file-option>] [--all] [--value=<value>] [--fixed-value] <name> <value>"),
> +	N_("git config unset [<file-option>] [--all] [--value=<value>] [--fixed-value] <name>"),
>  	NULL
>  };

Re: [PATCH v2 1/1] git-config.1: remove value from positional args in unset usage

[Patrick Steinhardt]

On Tue, Oct 08, 2024 at 11:20:48AM -0700, Junio C Hamano wrote:
> Josh Heinrichs <joshiheinrichs@gmail.com> writes:
> 
> > The synopsis for `git config unset` mentions two positional arguments:
> > `<name>` and `<value>`. While the first argument is correct, the second
> > is not. Users are expected to provide the value via `--value=<value>`.
> >
> > Remove the positional argument. The `--value=<value>` option is already
> > documented correctly, so this is all we need to do to fix the
> > documentation.
> >
> > Signed-off-by: Josh Heinrichs <joshiheinrichs@gmail.com>
> > ---
> >  Documentation/git-config.txt | 2 +-
> >  builtin/config.c             | 4 ++--
> >  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> Patrick, do you mind to forge your Reviewed-by for this one?
> 
> Will queue.  Thanks, both.

No, please feel free to add it.

    Reviewed-by: Patrick Steinhardt <ps@pks.im>

Patrick