threads / patch / 62277

patchRemove value from positional args in config unset usage

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

## tl;dr

7 messages between Oct 8, 2024 and Oct 9, 2024. Diffs are folded; open one to read it.

replies: 6people: 3as markdown or json

Josh Heinrichs· Oct 8, 2024, 04:07 UTC · lore

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
Josh Heinrichs· Oct 8, 2024, 04:07 UTC · re: Josh Heinrichs · lore

[PATCH 1/1] 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.
---
 Documentation/git-config.txt | 2 +-
 builtin/config.c             | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)
Show changes to 2 files +3 −3

Documentation/git-config.txt, builtin/config.c

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
Patrick Steinhardt· Oct 8, 2024, 12:37 UTC · re: Josh Heinrichs · lore

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

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
Josh Heinrichs· Oct 8, 2024, 14:29 UTC · re: Josh Heinrichs · lore

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

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
Josh Heinrichs· Oct 8, 2024, 14:29 UTC · re: Josh Heinrichs · lore

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

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(-)
Show changes to 2 files +3 −3

Documentation/git-config.txt, builtin/config.c

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
Junio C Hamano· Oct 8, 2024, 18:20 UTC · re: Josh Heinrichs · lore

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

Josh Heinrichs <joshiheinrichs@gmail.com> writes:
Show 13 quoted lines
> 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.
Show 34 quoted lines
> 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
>  };
Patrick Steinhardt· Oct 9, 2024, 03:37 UTC · re: Junio C Hamano · lore

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

On Tue, Oct 08, 2024 at 11:20:48AM -0700, Junio C Hamano wrote:
Show 19 quoted lines
> 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

← back to recent threads