threads / patch / 18342

patchconfig: test for --replace-all with one argument and fix documentation.

Subject: [PATCH] config: test for --replace-all with one argument and fix documentation.

## tl;dr

5 messages between Mar 17, 2009 and Mar 17, 2009. Diffs are folded; open one to read it.

replies: 4people: 4as markdown or json

Carlos Rica· Mar 17, 2009, 09:46 UTC · lore

Option --replace-all only allows at least two arguments, so documentation was needing to be updated accordingly. A test showing that the command fails with only one parameter is also provided.

Signed-off-by: Carlos Rica <jasampler@gmail.com>
---

This is applied on top of current pu, using the Felipe Contreras changes for adding parse-options to git-config.

 Documentation/git-config.txt |    2 +-
 builtin-config.c             |    2 +-
 t/t1300-repo-config.sh       |    9 ++++++++-
 3 files changed, 10 insertions(+), 3 deletions(-)
Show changes to 3 files +10 −3

Documentation/git-config.txt, builtin-config.c, t/t1300-repo-config.sh

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 82ce89e..7131ee3 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -11,7 +11,7 @@ SYNOPSIS
 [verse]
 'git config' [<file-option>] [type] [-z|--null] name [value [value_regex]]
 'git config' [<file-option>] [type] --add name value
-'git config' [<file-option>] [type] --replace-all name [value [value_regex]]
+'git config' [<file-option>] [type] --replace-all name value [value_regex]
 'git config' [<file-option>] [type] [-z|--null] --get name [value_regex]
 'git config' [<file-option>] [type] [-z|--null] --get-all name [value_regex]
 'git config' [<file-option>] [type] [-z|--null] --get-regexp name_regex [value_regex]
diff --git a/builtin-config.c b/builtin-config.c
index 1a3baa1..d8da72c 100644
--- a/builtin-config.c
+++ b/builtin-config.c
@@ -55,7 +55,7 @@ static struct option builtin_config_options[] = {
 	OPT_BIT(0, "get", &actions, "get value: name [value-regex]", ACTION_GET),
 	OPT_BIT(0, "get-all", &actions, "get all values: key [value-regex]", ACTION_GET_ALL),
 	OPT_BIT(0, "get-regexp", &actions, "get values for regexp: name-regex [value-regex]", ACTION_GET_REGEXP),
-	OPT_BIT(0, "replace-all", &actions, "replace all matching variables: name [value [value_regex]", ACTION_REPLACE_ALL),
+	OPT_BIT(0, "replace-all", &actions, "replace all matching variables: name value [value_regex]", ACTION_REPLACE_ALL),
 	OPT_BIT(0, "add", &actions, "adds a new variable: name value", ACTION_ADD),
 	OPT_BIT(0, "unset", &actions, "removes a variable: name [value-regex]", ACTION_UNSET),
 	OPT_BIT(0, "unset-all", &actions, "removes all matches: name [value-regex]", ACTION_UNSET_ALL),
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 3c06842..9c81e04 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -118,7 +118,14 @@ EOF
 
 test_expect_success 'multiple unset is correct' 'cmp .git/config expect'
 
-mv .git/config2 .git/config
+cp .git/config2 .git/config
+
+test_expect_success '--replace-all missing value' '
+	test_must_fail git config --replace-all beta.haha &&
+	test_cmp .git/config2 .git/config
+'
+
+rm .git/config2
 
 test_expect_success '--replace-all' \
 	'git config --replace-all beta.haha gamma'

-- 
1.6.0.5
Johannes Schindelin· Mar 17, 2009, 10:24 UTC · re: Carlos Rica · lore

Re: [PATCH] config: test for --replace-all with one argument and fix documentation.

Hi,
On Tue, 17 Mar 2009, Carlos Rica wrote:
Show 5 quoted lines
> Option --replace-all only allows at least two arguments, so
> documentation was needing to be updated accordingly. A test showing
> that the command fails with only one parameter is also provided.
> 
> Signed-off-by: Carlos Rica <jasampler@gmail.com>

Looks obviously correct to me. I am actually unsure if I can ACK this patch, as most of builtin-config.c does not look all that familiar to me anymore ;-)

Ciao, Dscho

Felipe Contreras· Mar 17, 2009, 10:41 UTC · re: Johannes Schindelin · lore

Re: [PATCH] config: test for --replace-all with one argument and fix documentation.

On Tue, Mar 17, 2009 at 12:24 PM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

Show 13 quoted lines
> Hi,
>
> On Tue, 17 Mar 2009, Carlos Rica wrote:
>
>> Option --replace-all only allows at least two arguments, so
>> documentation was needing to be updated accordingly. A test showing
>> that the command fails with only one parameter is also provided.
>>
>> Signed-off-by: Carlos Rica <jasampler@gmail.com>
>
> Looks obviously correct to me.  I am actually unsure if I can ACK this
> patch, as most of builtin-config.c does not look all that familiar to me
> anymore ;-)
Hehe... interesting, my first possibility of ack'ing :D (I guess)
Acked-by: Felipe Contreras <felipe.contreras@gmail.com>
-- 
Felipe Contreras
Uwe Kleine-König· Mar 17, 2009, 11:02 UTC · re: Felipe Contreras · lore

Re: [PATCH] config: test for --replace-all with one argument and fix documentation.

Hello,
On Tue, Mar 17, 2009 at 12:41:29PM +0200, Felipe Contreras wrote:
Show 19 quoted lines
> On Tue, Mar 17, 2009 at 12:24 PM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> > Hi,
> >
> > On Tue, 17 Mar 2009, Carlos Rica wrote:
> >
> >> Option --replace-all only allows at least two arguments, so
> >> documentation was needing to be updated accordingly. A test showing
> >> that the command fails with only one parameter is also provided.
> >>
> >> Signed-off-by: Carlos Rica <jasampler@gmail.com>
> >
> > Looks obviously correct to me.  I am actually unsure if I can ACK this
> > patch, as most of builtin-config.c does not look all that familiar to me
> > anymore ;-)
> 
> Hehe... interesting, my first possibility of ack'ing :D (I guess)
> 
> Acked-by: Felipe Contreras <felipe.contreras@gmail.com>

It's not nice to ack in public that someone else doesn't know something anymore. :-)

SCNR Uwe

-- 
Pengutronix e.K.                              | Uwe Kleine-König            |
Industrial Linux Solutions                    | http://www.pengutronix.de/  |
Carlos Rica· Mar 17, 2009, 11:12 UTC · re: Johannes Schindelin · lore

Re: [PATCH] config: test for --replace-all with one argument and fix documentation.

On Tue, Mar 17, 2009 at 11:24 AM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

Show 13 quoted lines
> Hi,
>
> On Tue, 17 Mar 2009, Carlos Rica wrote:
>
>> Option --replace-all only allows at least two arguments, so
>> documentation was needing to be updated accordingly. A test showing
>> that the command fails with only one parameter is also provided.
>>
>> Signed-off-by: Carlos Rica <jasampler@gmail.com>
>
> Looks obviously correct to me.  I am actually unsure if I can ACK this
> patch, as most of builtin-config.c does not look all that familiar to me
> anymore ;-)

Thank you Johannes, I think that you don't need to do it in this patch, I talked before with Junio and Felipe for this, and I think that Junio is also following the whole change (my change here is not comparable with the work of Felipe).

I just added you because Felipe Contreras said that you made some suggestions to make this use of parse options possible.

See you!

← back to recent threads