threads / patch / 49561

patchTypo `dashes ?` -> `dashes?`

Subject: [PATCH] Typo `dashes ?` -> `dashes?`

## tl;dr

8 messages between Oct 14, 2018 and Feb 5, 2020. Diffs are folded; open one to read it.

replies: 7people: 3as markdown or json

Jacques Bodin-Hullin· Oct 14, 2018, 19:44 UTC · lore
---
 parse-options.c          | 4 ++--
 t/t0040-parse-options.sh | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)
Show changes to 2 files +4 −5

parse-options.c, t/t0040-parse-options.sh

diff --git a/parse-options.c b/parse-options.c
index 3b874a83a0c89..88512212392ae 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -352,7 +352,7 @@ static void check_typos(const char *arg, const struct option *options)
 		return;
 
 	if (starts_with(arg, "no-")) {
-		error ("did you mean `--%s` (with two dashes ?)", arg);
+		error ("did you mean `--%s` (with two dashes?)", arg);
 		exit(129);
 	}
 
@@ -360,7 +360,7 @@ static void check_typos(const char *arg, const struct option *options)
 		if (!options->long_name)
 			continue;
 		if (starts_with(options->long_name, arg)) {
-			error ("did you mean `--%s` (with two dashes ?)", arg);
+			error ("did you mean `--%s` (with two dashes?)", arg);
 			exit(129);
 		}
 	}
diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index 5b0560fa20e34..16fd333bd3895 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -222,7 +222,7 @@ test_expect_success 'non ambiguous option (after two options it abbreviates)' '
 '
 
 cat >typo.err <<\EOF
-error: did you mean `--boolean` (with two dashes ?)
+error: did you mean `--boolean` (with two dashes?)
 EOF
 
 test_expect_success 'detect possible typos' '
@@ -232,7 +232,7 @@ test_expect_success 'detect possible typos' '
 '
 
 cat >typo.err <<\EOF
-error: did you mean `--ambiguous` (with two dashes ?)
+error: did you mean `--ambiguous` (with two dashes?)
 EOF
 
 test_expect_success 'detect possible typos' '

--
https://github.com/git/git/pull/540
Jeff King· Oct 15, 2018, 17:20 UTC · re: Jacques Bodin-Hullin · lore

Re: [PATCH] Typo `dashes ?` -> `dashes?`

On Sun, Oct 14, 2018 at 07:44:58PM +0000, Jacques Bodin-Hullin wrote:
Show 10 quoted lines
> diff --git a/parse-options.c b/parse-options.c
> index 3b874a83a0c89..88512212392ae 100644
> --- a/parse-options.c
> +++ b/parse-options.c
> @@ -352,7 +352,7 @@ static void check_typos(const char *arg, const struct option *options)
>  		return;
>  
>  	if (starts_with(arg, "no-")) {
> -		error ("did you mean `--%s` (with two dashes ?)", arg);
> +		error ("did you mean `--%s` (with two dashes?)", arg);

I agree the extra space in the original is stylistically weird, and your suggestion is an improvement. However, I think this is really a question "did you mean..." with a parenthetical phrase. Most style guides would recommend putting the punctuation outside, like:

  error: did you mean `--foo` (with two dashes)?

Speaking of style, the extra space between "error" and "(" does not match our usual coding style. It might be worth removing while we're touching these lines.

-Peff
Jacques Bodin-Hullin· Nov 16, 2018, 22:58 UTC · re: Jacques Bodin-Hullin · lore

[PATCH-2] Change writing style

---
 parse-options.c          | 4 ++--
 t/t0040-parse-options.sh | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)
Show changes to 2 files +4 −5

parse-options.c, t/t0040-parse-options.sh

diff --git a/parse-options.c b/parse-options.c
index 3b874a83a0c89..f5ef24155950b 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -352,7 +352,7 @@ static void check_typos(const char *arg, const struct option *options)
 		return;
 
 	if (starts_with(arg, "no-")) {
-		error ("did you mean `--%s` (with two dashes ?)", arg);
+		error("did you mean `--%s` (with two dashes)?", arg);
 		exit(129);
 	}
 
@@ -360,7 +360,7 @@ static void check_typos(const char *arg, const struct option *options)
 		if (!options->long_name)
 			continue;
 		if (starts_with(options->long_name, arg)) {
-			error ("did you mean `--%s` (with two dashes ?)", arg);
+			error("did you mean `--%s` (with two dashes)?", arg);
 			exit(129);
 		}
 	}
diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index 5b0560fa20e34..c442f9ae15ff8 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -222,7 +222,7 @@ test_expect_success 'non ambiguous option (after two options it abbreviates)' '
 '
 
 cat >typo.err <<\EOF
-error: did you mean `--boolean` (with two dashes ?)
+error: did you mean `--boolean` (with two dashes)?
 EOF
 
 test_expect_success 'detect possible typos' '
@@ -232,7 +232,7 @@ test_expect_success 'detect possible typos' '
 '
 
 cat >typo.err <<\EOF
-error: did you mean `--ambiguous` (with two dashes ?)
+error: did you mean `--ambiguous` (with two dashes)?
 EOF
 
 test_expect_success 'detect possible typos' '

--
https://github.com/git/git/pull/540
Junio C Hamano· Nov 17, 2018, 09:04 UTC · re: Jacques Bodin-Hullin · lore

Re: [PATCH-2] Change writing style

Jacques Bodin-Hullin <j.bodinhullin@monsieurbiz.com> writes:
> Subject: Re: [PATCH-2] Change writing style
Let's do the usual "<area>: summary" instead.  Perhaps
    Subject: parse-options: lose an unnecessary space in an error message

It is obvious why it is done, so I do not see a need for any other description in the body of the message for this change.

We still need the patch signed-off.  cf. Documentation/SubmittingPatches
> ---
>  parse-options.c          | 4 ++--
>  t/t0040-parse-options.sh | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
All changes look good to me.
Show 47 quoted lines
> diff --git a/parse-options.c b/parse-options.c
> index 3b874a83a0c89..f5ef24155950b 100644
> --- a/parse-options.c
> +++ b/parse-options.c
> @@ -352,7 +352,7 @@ static void check_typos(const char *arg, const struct option *options)
>  		return;
>  
>  	if (starts_with(arg, "no-")) {
> -		error ("did you mean `--%s` (with two dashes ?)", arg);
> +		error("did you mean `--%s` (with two dashes)?", arg);
>  		exit(129);
>  	}
>  
> @@ -360,7 +360,7 @@ static void check_typos(const char *arg, const struct option *options)
>  		if (!options->long_name)
>  			continue;
>  		if (starts_with(options->long_name, arg)) {
> -			error ("did you mean `--%s` (with two dashes ?)", arg);
> +			error("did you mean `--%s` (with two dashes)?", arg);
>  			exit(129);
>  		}
>  	}
> diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
> index 5b0560fa20e34..c442f9ae15ff8 100755
> --- a/t/t0040-parse-options.sh
> +++ b/t/t0040-parse-options.sh
> @@ -222,7 +222,7 @@ test_expect_success 'non ambiguous option (after two options it abbreviates)' '
>  '
>  
>  cat >typo.err <<\EOF
> -error: did you mean `--boolean` (with two dashes ?)
> +error: did you mean `--boolean` (with two dashes)?
>  EOF
>  
>  test_expect_success 'detect possible typos' '
> @@ -232,7 +232,7 @@ test_expect_success 'detect possible typos' '
>  '
>  
>  cat >typo.err <<\EOF
> -error: did you mean `--ambiguous` (with two dashes ?)
> +error: did you mean `--ambiguous` (with two dashes)?
>  EOF
>  
>  test_expect_success 'detect possible typos' '
>
> --
> https://github.com/git/git/pull/540
Jacques Bodin-Hullin· Feb 5, 2020, 13:07 UTC · re: Jacques Bodin-Hullin · lore

[PATCH] parse-options: lose an unnecessary space in an error message

Signed-off-by: Jacques Bodin-Hullin <j.bodinhullin@monsieurbiz.com>
---
 parse-options.c          | 4 ++--
 t/t0040-parse-options.sh | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)
Show changes to 2 files +4 −5

parse-options.c, t/t0040-parse-options.sh

diff --git a/parse-options.c b/parse-options.c
index b42f54d48b96c..71dbe84d896b4 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -420,7 +420,7 @@ static void check_typos(const char *arg, const struct option *options)
 		return;
 
 	if (starts_with(arg, "no-")) {
-		error(_("did you mean `--%s` (with two dashes ?)"), arg);
+		error(_("did you mean `--%s` (with two dashes)?"), arg);
 		exit(129);
 	}
 
@@ -428,7 +428,7 @@ static void check_typos(const char *arg, const struct option *options)
 		if (!options->long_name)
 			continue;
 		if (starts_with(options->long_name, arg)) {
-			error(_("did you mean `--%s` (with two dashes ?)"), arg);
+			error(_("did you mean `--%s` (with two dashes)?"), arg);
 			exit(129);
 		}
 	}
diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index 705a136ed92c9..9d7c7fdaa2af1 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -242,7 +242,7 @@ test_expect_success 'Alias options do not contribute to abbreviation' '
 '
 
 cat >typo.err <<\EOF
-error: did you mean `--boolean` (with two dashes ?)
+error: did you mean `--boolean` (with two dashes)?
 EOF
 
 test_expect_success 'detect possible typos' '
@@ -252,7 +252,7 @@ test_expect_success 'detect possible typos' '
 '
 
 cat >typo.err <<\EOF
-error: did you mean `--ambiguous` (with two dashes ?)
+error: did you mean `--ambiguous` (with two dashes)?
 EOF
 
 test_expect_success 'detect possible typos' '

--
https://github.com/git/git/pull/540
Junio C Hamano· Feb 5, 2020, 18:49 UTC · re: Jacques Bodin-Hullin · lore

Re: [PATCH] parse-options: lose an unnecessary space in an error message

Jacques Bodin-Hullin <j.bodinhullin@monsieurbiz.com> writes:
Show 16 quoted lines
> Signed-off-by: Jacques Bodin-Hullin <j.bodinhullin@monsieurbiz.com>
> ---
>  parse-options.c          | 4 ++--
>  t/t0040-parse-options.sh | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/parse-options.c b/parse-options.c
> index b42f54d48b96c..71dbe84d896b4 100644
> --- a/parse-options.c
> +++ b/parse-options.c
> @@ -420,7 +420,7 @@ static void check_typos(const char *arg, const struct option *options)
>  		return;
>  
>  	if (starts_with(arg, "no-")) {
> -		error(_("did you mean `--%s` (with two dashes ?)"), arg);
> +		error(_("did you mean `--%s` (with two dashes)?"), arg);

Makes sense. Not just there is an extra SP before the question mark, the question mark is about the whole sentence, so pushing it out of the parentheses also is a good change.

We might even want to lose the parentheses altogether, i.e.
	did you mean `--%s` with two dashes?
Hmm?
Thanks.
Jacques Bodin-Hullin· Feb 5, 2020, 20:11 UTC · re: Junio C Hamano · lore

Re: [PATCH] parse-options: lose an unnecessary space in an error message

Hi,

Actually I prefer to keep the parentheses, it makes better sense to me. It feels correct with them when I speak it loudly, incorrect without.

BR,
Jacques
-- 
Monsieur Biz
@jacquesbh
+33 (0)6 75 54 11 97
GPG : https://keybase.io/jacquesbh


Le mer. 5 févr. 2020 à 19:49, Junio C Hamano <gitster@pobox.com> a écrit :
>
> Jacques Bodin-Hullin <j.bodinhullin@monsieurbiz.com> writes:
>
> > Signed-off-by: Jacques Bodin-Hullin <j.bodinhullin@monsieurbiz.com>
> > ---
> >  parse-options.c          | 4 ++--
> >  t/t0040-parse-options.sh | 4 ++--
> >  2 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/parse-options.c b/parse-options.c
> > index b42f54d48b96c..71dbe84d896b4 100644
> > --- a/parse-options.c
> > +++ b/parse-options.c
> > @@ -420,7 +420,7 @@ static void check_typos(const char *arg, const struct option *options)
> >               return;
> >
> >       if (starts_with(arg, "no-")) {
> > -             error(_("did you mean `--%s` (with two dashes ?)"), arg);
> > +             error(_("did you mean `--%s` (with two dashes)?"), arg);
>
> Makes sense.  Not just there is an extra SP before the question
> mark, the question mark is about the whole sentence, so pushing
> it out of the parentheses also is a good change.
>
> We might even want to lose the parentheses altogether, i.e.
>
>         did you mean `--%s` with two dashes?
>
> Hmm?
>
> Thanks.
Jeff King· Oct 17, 2018, 08:23 UTC · lore

Re: [PATCH] Typo `dashes ?` -> `dashes?`

On Mon, Oct 15, 2018 at 09:31:54PM +0200, Jacques Bodin-Hullin wrote:
Show 7 quoted lines
> I've just updated the PR with the `error(` change.
> 
> Also I did the change for the question mark at the end, because you are
> right, when we read it, it sounds better.
> 
> Do I need to put back the patch in this thread? It's here:
> https://patch-diff.githubusercontent.com/raw/git/git/pull/540.patch

The commit looks good, though you may want to expand the commit message a little (it's not really a typo, but more of a style fix).

The next step would normally be to send the patch to the list (ideally in this thread), with "[PATCH v2]" in the subject.

It looks like you're using SubmitGit. I don't remember offhand how it works for sending re-rolls like this, but I think it's possible. If you run into trouble, you might try instead using GitGitGadget, which is a similar PR->mail gateway that is a little more featureful:

  https://github.com/gitgitgadget/gitgitgadget
-Peff

← back to recent threads