Re: [PATCH 6/6] parse-options: automatically infer PARSE_OPT_LITERAL_ARGHELP

[Kerry, Richard]

s/angular brackets/angle brackets/

I've never seen these called "angular brackets".  Maybe a non-native English 
speaker issue.



Regards,
Richard.

PS. Please excuse my sending this twice, I don't seem to have default settings 
that are compatible with the list.




Richard Kerry
BNCS Engineer, SI SOL Telco & Media Vertical  Practice
M: +44 (0)7812 325518
2nd Floor, MidCity Place, 71 High Holborn, London, WC1V 6EA
richard.ke...@atos.net



 This e-mail and the documents attached are confidential and intended solely 
for the addressee; it may also be privileged. If you receive  this e-mail in 
error, please notify the sender immediately and destroy it. As its integrity 
cannot be secured on the Internet, the Atos group liability cannot be triggered 
for the message content. Although the sender endeavours to maintain a computer 
virus-free  network, the sender does not warrant that this transmission is 
virus-free and will not be liable for any damages resulting from any virus 
transmitted.




From: git-ow...@vger.kernel.org  on behalf of René 
Scharfe 
Sent: 02 August 2018 20:18
To: Jeff King; Junio C Hamano
Cc: Ævar Arnfjörð Bjarmason; git@vger.kernel.org
Subject: [PATCH 6/6] parse-options: automatically infer 
PARSE_OPT_LITERAL_ARGHELP


Parseopt wraps argument help strings in a pair of angular brackets by
default, to tell users that they need to replace it with an actual
value.  This is useful in most cases, because most option arguments
are indeed single values of a certain type.  The option
PARSE_OPT_LITERAL_ARGHELP needs to be used in option definitions with
arguments that have multiple parts or are literal strings.

Stop adding these angular brackets if special characters are present,
as they indicate that we don't deal with a simple placeholder.  This
simplifies the code a bit and makes defining special options slightly
easier.

Remove the flag PARSE_OPT_LITERAL_ARGHELP in the cases where the new
and more cautious handling suffices.

Signed-off-by: Rene Scharfe 
---
The patch to add PARSE_OPT_LITERAL_ARGHELP for push --force-with-lease
should be applied before this one.

 builtin/add.c  | 5 ++---
 builtin/pack-objects.c | 2 +-
 builtin/read-tree.c| 2 +-
 builtin/send-pack.c| 3 +--
 builtin/shortlog.c | 3 +--
 builtin/show-branch.c  | 2 +-
 builtin/update-index.c | 2 +-
 builtin/write-tree.c   | 5 ++---
 parse-options.c| 3 ++-
 9 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/builtin/add.c b/builtin/add.c
index 84bfec9b73..ba1ff5689d 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -304,9 +304,8 @@ static struct option builtin_add_options[] = {
 OPT_BOOL( 0 , "refresh", &refresh_only, N_("don't add, only refresh 
the index")),
 OPT_BOOL( 0 , "ignore-errors", &ignore_add_errors, N_("just skip files 
which cannot be added because of errors")),
 OPT_BOOL( 0 , "ignore-missing", &ignore_missing, N_("check if - even 
missing - files are ignored in dry run")),
-   { OPTION_STRING, 0, "chmod", &chmod_arg, "(+|-)x",
- N_("override the executable bit of the listed files"),
- PARSE_OPT_LITERAL_ARGHELP },
+   OPT_STRING(0, "chmod", &chmod_arg, "(+|-)x",
+  N_("override the executable bit of the listed files")),
 OPT_HIDDEN_BOOL(0, "warn-embedded-repo", &warn_on_embedded_repo,
 N_("warn when adding an embedded repository")),
 OPT_END(),
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 3a5d1fa317..b2323613bc 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -3112,7 +3112,7 @@ int cmd_pack_objects(int argc, const char **argv, const 
char *prefix)
  N_("similar to --all-progress when progress meter is 
shown")),
 { OPTION_CALLBACK, 0, "index-version", NULL, 
N_("[,]"),
   N_("write the pack index file in the specified idx format 
version"),
- PARSE_OPT_LITERAL_ARGHELP, option_parse_index_version },
+ 0, option_parse_index_version },
 OPT_MAGNITUDE(0, "max-pack-size", &pack_size_limit,
   N_("maximum size of each output pack file")),
 OPT_BOOL(0, "local", &local,
diff --git a/builtin/read-tree.c b/builtin/read-tree.c
index ebc43eb805..fbbc98e516 100644
--- a/builtin/read-tree.c
+++ b/builtin/read-tree.c
@@ -133,7 +133,7 @@ int cmd_read_tree(int argc, const char **argv, const char 
*unused_prefix)
  N_("same as -m, but discard unmerged entries")),
 { OPTION_STRING, 0, "prefix", &opts.prefix, 
N_("/"),
   N_("read the tree into the index under /"),
- PARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP },
+ PARSE_OPT_NONEG },
 OPT_BOOL('u', NULL, &opts.update,
  N_("update working tree with merge result")),
 { OPTION_CALLBACK, 0, "exclude-per-directory",

Re: [PATCH 6/6] parse-options: automatically infer PARSE_OPT_LITERAL_ARGHELP

[Kerry, Richard]

s/angular brackets/angle brackets/



I've never seen these called "angular brackets".


Richard Kerry
BNCS Engineer, SI SOL Telco & Media Vertical Practice
M: +44 (0)7812 325518
2nd Floor, MidCity Place, 71 High Holborn, London, WC1V 6EA
richard.ke...@atos.net

This e-mail and the documents attached are confidential and intended solely for 
the addressee; it may also be privileged. If you receive this e-mail in error, 
please notify the sender immediately and destroy it. As its integrity cannot be 
secured on the Internet, the Atos group liability cannot be triggered for the 
message content. Although the sender endeavours to maintain a computer 
virus-free network, the sender does not warrant that this transmission is 
virus-free and will not be liable for any damages resulting from any virus 
transmitted.



From: git-ow...@vger.kernel.org  on behalf of René 
Scharfe 
Sent: 02 August 2018 20:18
To: Jeff King; Junio C Hamano
Cc: Ævar Arnfjörð Bjarmason; git@vger.kernel.org
Subject: [PATCH 6/6] parse-options: automatically infer 
PARSE_OPT_LITERAL_ARGHELP

Parseopt wraps argument help strings in a pair of angular brackets by
default, to tell users that they need to replace it with an actual
value.  This is useful in most cases, because most option arguments
are indeed single values of a certain type.  The option
PARSE_OPT_LITERAL_ARGHELP needs to be used in option definitions with
arguments that have multiple parts or are literal strings.

Stop adding these angular brackets if special characters are present,
as they indicate that we don't deal with a simple placeholder.  This
simplifies the code a bit and makes defining special options slightly
easier.

Remove the flag PARSE_OPT_LITERAL_ARGHELP in the cases where the new
and more cautious handling suffices.

Signed-off-by: Rene Scharfe 
---
The patch to add PARSE_OPT_LITERAL_ARGHELP for push --force-with-lease
should be applied before this one.

 builtin/add.c  | 5 ++---
 builtin/pack-objects.c | 2 +-
 builtin/read-tree.c| 2 +-
 builtin/send-pack.c| 3 +--
 builtin/shortlog.c | 3 +--
 builtin/show-branch.c  | 2 +-
 builtin/update-index.c | 2 +-
 builtin/write-tree.c   | 5 ++---
 parse-options.c| 3 ++-
 9 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/builtin/add.c b/builtin/add.c
index 84bfec9b73..ba1ff5689d 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -304,9 +304,8 @@ static struct option builtin_add_options[] = {
 OPT_BOOL( 0 , "refresh", &refresh_only, N_("don't add, only refresh 
the index")),
 OPT_BOOL( 0 , "ignore-errors", &ignore_add_errors, N_("just skip files 
which cannot be added because of errors")),
 OPT_BOOL( 0 , "ignore-missing", &ignore_missing, N_("check if - even 
missing - files are ignored in dry run")),
-   { OPTION_STRING, 0, "chmod", &chmod_arg, "(+|-)x",
- N_("override the executable bit of the listed files"),
- PARSE_OPT_LITERAL_ARGHELP },
+   OPT_STRING(0, "chmod", &chmod_arg, "(+|-)x",
+  N_("override the executable bit of the listed files")),
 OPT_HIDDEN_BOOL(0, "warn-embedded-repo", &warn_on_embedded_repo,
 N_("warn when adding an embedded repository")),
 OPT_END(),
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 3a5d1fa317..b2323613bc 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -3112,7 +3112,7 @@ int cmd_pack_objects(int argc, const char **argv, const 
char *prefix)
  N_("similar to --all-progress when progress meter is 
shown")),
 { OPTION_CALLBACK, 0, "index-version", NULL, 
N_("[,]"),
   N_("write the pack index file in the specified idx format 
version"),
- PARSE_OPT_LITERAL_ARGHELP, option_parse_index_version },
+ 0, option_parse_index_version },
 OPT_MAGNITUDE(0, "max-pack-size", &pack_size_limit,
   N_("maximum size of each output pack file")),
 OPT_BOOL(0, "local", &local,
diff --git a/builtin/read-tree.c b/builtin/read-tree.c
index ebc43eb805..fbbc98e516 100644
--- a/builtin/read-tree.c
+++ b/builtin/read-tree.c
@@ -133,7 +133,7 @@ int cmd_read_tree(int argc, const char **argv, const char 
*unused_prefix)
  N_("same as -m, but discard unmerged entries")),
 { OPTION_STRING, 0, "prefix", &opts.prefix, 
N_("/"),
   N_("read the tree into the index under /"),
- PARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP },
+ PARSE_OPT_NONEG },
 OPT_BOOL('u', NULL, &opts.update,
  N_("update working tree with merge result")),
 { OPTION_CALLBACK, 0, "exclude-per-directory", &opts,
diff --git a/builtin

RE: [PATCH v2 6/9] rebase -i: update functions to use a flags parameter

[Kerry, Richard]

Indeed so.
In which case it is short for "selling short", or possibly "short selling".

Of course a little searching shows that "shorted" could mean some other things, 
including possibly the meaning originally suggested.
Nevertheless it seems to me that "shortened" is the most appropriate word in 
modern English.

R.


Richard Kerry
BNCS Engineer, SI SOL Telco & Media Vertical Practice

T: +44 (0)20 3618 2669
M: +44 (0)7812 325518
Lync: +44 (0) 20 3618 0778
Room G300, Stadium House, Wood Lane, London, W12 7TA
richard.ke...@atos.net




> -Original Message-
> From: Junio C Hamano [mailto:gits...@pobox.com]
> Sent: Tuesday, December 05, 2017 4:06 PM
> To: Kerry, Richard 
> Cc: git@vger.kernel.org; Johannes Schindelin
> ; p...@peff.net; liam Beguin
> 
> Subject: Re: [PATCH v2 6/9] rebase -i: update functions to use a flags
> parameter
>
> "Kerry, Richard"  writes:
>
> > "Shorted" is what happens when you put a piece of wire across the
> terminals of a battery ... (bang, smoke, etc).
> > It's short for "short-circuited".
>
> Or it is what you do to something that you sell and that you yet do not own,
> expecting that you can later buy it cheaper, allowing you to pocket the
> difference ;-).
>

Atos, Atos Consulting, Worldline and Canopy The Open Cloud Company are trading 
names used by the Atos group. The following trading entities are registered in 
England and Wales: Atos IT Services UK Limited (registered number 01245534), 
Atos Consulting Limited (registered number 04312380), Atos Worldline UK Limited 
(registered number 08514184) and Canopy The Open Cloud Company Limited 
(registration number 08011902). The registered office for each is at 4 Triton 
Square, Regent’s Place, London, NW1 3HG.The VAT No. for each is: GB232327983.

This e-mail and the documents attached are confidential and intended solely for 
the addressee, and may contain confidential or privileged information. If you 
receive this e-mail in error, you are not authorised to copy, disclose, use or 
retain it. Please notify the sender immediately and delete this email from your 
systems. As emails may be intercepted, amended or lost, they are not secure. 
Atos therefore can accept no liability for any errors or their content. 
Although Atos endeavours to maintain a virus-free network, we do not warrant 
that this transmission is virus-free and can accept no liability for any 
damages resulting from any virus transmitted. The risks are deemed to be 
accepted by everyone who communicates with Atos by email.

RE: [PATCH v2 6/9] rebase -i: update functions to use a flags parameter

[Kerry, Richard]

"Shorted" is what happens when you put a piece of wire across the terminals of 
a battery ... (bang, smoke, etc).
It's short for "short-circuited".
Yes, I think you mean "shortened" in this case.

Regards,
Richard.



Richard Kerry
BNCS Engineer, SI SOL Telco & Media Vertical Practice

T: +44 (0)20 3618 2669
M: +44 (0)7812 325518
Lync: +44 (0) 20 3618 0778
Room G300, Stadium House, Wood Lane, London, W12 7TA
richard.ke...@atos.net




> -Original Message-
> From: git-ow...@vger.kernel.org [mailto:git-ow...@vger.kernel.org] On
> Behalf Of Junio C Hamano
> Sent: Tuesday, December 05, 2017 12:37 PM
> To: liam Beguin 
> Cc: Johannes Schindelin ;
> git@vger.kernel.org; p...@peff.net
> Subject: Re: [PATCH v2 6/9] rebase -i: update functions to use a flags
> parameter
>
> liam Beguin  writes:
>
> > I'll change it to TODO_LIST_SHORTED_IDs. TODO_LIST_SHORTED_INSNS
> would
> > suggest the flag changes both parts of the todo.
>
> I am not a native speaker, but SHORTED does not sound like a right phrase.
> When you make something shorter, that thing is "shortened", not "shorted".

Atos, Atos Consulting, Worldline and Canopy The Open Cloud Company are trading 
names used by the Atos group. The following trading entities are registered in 
England and Wales: Atos IT Services UK Limited (registered number 01245534), 
Atos Consulting Limited (registered number 04312380), Atos Worldline UK Limited 
(registered number 08514184) and Canopy The Open Cloud Company Limited 
(registration number 08011902). The registered office for each is at 4 Triton 
Square, Regent’s Place, London, NW1 3HG.The VAT No. for each is: GB232327983.

This e-mail and the documents attached are confidential and intended solely for 
the addressee, and may contain confidential or privileged information. If you 
receive this e-mail in error, you are not authorised to copy, disclose, use or 
retain it. Please notify the sender immediately and delete this email from your 
systems. As emails may be intercepted, amended or lost, they are not secure. 
Atos therefore can accept no liability for any errors or their content. 
Although Atos endeavours to maintain a virus-free network, we do not warrant 
that this transmission is virus-free and can accept no liability for any 
damages resulting from any virus transmitted. The risks are deemed to be 
accepted by everyone who communicates with Atos by email.

RE: [PATCH v2 1/3] usability: don't ask questions if no reply is required

[Kerry, Richard]

Some more grammar/usage notes .

> -Original Message-
> From: git-ow...@vger.kernel.org [mailto:git-ow...@vger.kernel.org] On
> Behalf Of Junio C Hamano
> Sent: Thursday, May 11, 2017 4:16 AM
> To: Jean-Noel Avila 
> Cc: git@vger.kernel.org; rashmipa...@gmail.com
> Subject: Re: [PATCH v2 1/3] usability: don't ask questions if no reply is
> required
>
> Jean-Noel Avila  writes:
>
> > diff --git a/builtin/am.c b/builtin/am.c index a95dd8b4e..f5afa438d
> > 100644
> > --- a/builtin/am.c
> > +++ b/builtin/am.c
> > @@ -1312,7 +1312,7 @@ static int parse_mail(struct am_state *state, const
> char *mail)
> > }
> >
> > if (is_empty_file(am_path(state, "patch"))) {
> > -   printf_ln(_("Patch is empty. Was it split wrong?"));
> > +   printf_ln(_("Patch is empty. It may have been split wrong."));
> > die_user_resolve(state);
> > }
>
> While I do not belong to "we should feel free to ask rhetorical questions"
> camp, I do not mind this particular rewrite.  An obvious alternative is just 
> to
> stop the sentence with "Patch is empty."
>
> At this point in the code, we do not even know why we are seeing an empty
> patch, and "perhaps it was incorrectly split" is not a particularly useful 
> idle
> speculation that would help the user who sees it.

s/split wrong/split wrongly/
Though the further discussion suggests that part of the phrase might best be 
removed entirely.


> > @@ -1940,7 +1940,7 @@ static void am_resolve(struct am_state *state)
> >
> > if (unmerged_cache()) {
> > printf_ln(_("You still have unmerged paths in your index.\n"
> > -   "Did you forget to use 'git add'?"));
> > +   "You might want to use 'git add' on them."));
>
> This case is *not* an "rhetorical question is the most succinct way to convey
> the information" situation; I think this rewrite is a definite improvement.
> "You might want to 'git add' them" may be more succinct, though.

"You might want to use 'git add' on them."
It isn't about what you *want* to use, it's what you *need* to use, isn't it?  
And I'm not happy about "on them".  I'm not sure quite why, but the phrasing 
seems odd.
How about "You might need to use 'git add'.", or "You might need to use 'git 
add' first.", or "'git add' needs to be used to add files." ,  or "'git add' 
needs to be used before any other git command may be used.".


> > diff --git a/builtin/checkout.c b/builtin/checkout.c index
> > bfa5419f3..05037b9b6 100644
> > --- a/builtin/checkout.c
> > +++ b/builtin/checkout.c
> > @@ -1287,7 +1287,7 @@ int cmd_checkout(int argc, const char **argv,
> const char *prefix)
> >  */
> > if (opts.new_branch && argc == 1)
> > die(_("Cannot update paths and switch to branch '%s'
> at the same time.\n"
> > - "Did you intend to checkout '%s' which can not be
> resolved as commit?"),
> > + "'%s' can not be resolved as commit, but it
> should."),

> I am not sure a firm statement "but it should" is an improvement.
> This message is given when the user says:
>
> $ git checkout -b newone naster
>
> And "but it should" is appropriate when it is a mistyped "I want to create and
> checkout 'newone' branch at the same commit as 'master'
> branch", i.e.
>
> $ git checkout -b newone master
>
> The reason why the message begins with "Cannot update paths and ..."
> is because it could be a mistyped "I want to grab the file 'naster'
> out of 'newone' branch", i.e. the user meant to say this:
>
> $ git checkout newone naster
>
> IOW, the current error message is hedging its bets, because it does not want
> to exclude the possibility that "-b" is there by mistake (as opposed to 
> 'naster'
> is the typo).
>
> If we ignore that possibility and assume that 'naster' is the typo (iow, the
> user did mean "-b"), then your updated message makes sense.  But if we
> commit to "the user meant -b", we could make the message even more
> helpful by being more direct, e.g.
>
>   die("'%s' is not a commit and a branch '%s' cannot be created from
> it",
>   argv[0], opts.new_branch);

"'%s' can not be resolved as commit, but it should."
It should what ?  Be resolved, or commit?  Or something else?
The further comments above suggest that it might even be just "'%s' can not be 
resolved."


> > diff --git a/help.c b/help.c
> > index bc6cd19cf..4658a55c6 100644
> > --- a/help.c
> > +++ b/help.c
> > @@ -411,8 +411,8 @@ const char *help_unknown_cmd(const char *cmd)
> >
> > if (SIMILAR_ENOUGH(best_similarity)) {
> > fprintf_ln(stderr,
> > -  Q_("\nDid you mean this?",
> > - "\nDid you mean one of these?",
> > +  Q_("\nThe most approaching command is",
> > + "\nThe most approaching commands are",
> >n));
>
> With "closest" or "most similar", as others pointed out, I think th

RE: [PATCH v2 1/3] usability: don't ask questions if no reply is required

[Kerry, Richard]

My point was to ensure that where English is used on-screen it should make 
sense, which in this particular case it didn't (a French idiom which, on using 
an automatic translator, didn't make sense in English).  The same of course 
applies to other languages used on-screen.

I agree about ensuring that the application doesn't elicit a response that it 
won't, or can't, actually handle.  A rhetorical question is fine, so long as it 
is clear that the program won't accept any further input.

Though I don't agree about the issue of the length of words, as presented to a 
non-native speaker.  Sometimes a longer word can be very specific in its 
meaning, and can be looked up in a dictionary if the reader is not familiar 
with it.  Sometimes using shorter words can result in a less clear meaning, or 
perhaps be an idiomatic usage, which might be missed by a non-native speaker.

Regards,
Richard.




Richard Kerry
BNCS Engineer, SI SOL Telco & Media Vertical Practice
T: +44 (0)20 3618 2669
M: +44 (0)7812 325518
4 Triton Square, Regent’s Place, London NW1 3HG
richard.ke...@atos.net


This e-mail and the documents attached are confidential and intended solely for 
the addressee; it may also be privileged. If you receive this e-mail in error, 
please notify the sender immediately and destroy it. As its integrity cannot be 
secured on the Internet, the Atos group liability cannot be triggered for the 
message content. Although the sender endeavours to maintain a computer 
virus-free network, the sender does not warrant that this transmission is 
virus-free and will not be liable for any damages resulting from any virus 
transmitted.


From: Ævar Arnfjörð Bjarmason [ava...@gmail.com]
Sent: 04 May 2017 10:09
To: Kerry, Richard
Cc: git@vger.kernel.org
Subject: Re: [PATCH v2 1/3] usability: don't ask questions if no reply is 
required

On Thu, May 4, 2017 at 10:52 AM, Kerry, Richard  wrote:
>
> May I suggest that " The most approaching commands" doesn't make much sense 
> as English (I don't think a command can "approach").
> Perhaps it should be " The most appropriate commands".

I had the same concern, saying "appropriate" is IMO also confusing.
The point of this UI is not to point out what you should be running,
which "appropriate" implies, but just "we couldn't find what you
meant, did you mean one of these?".

I think nothing needs to change here. The whole premise here is that a
program should never ask a question when you can't give an answer, I
think that's nonsense. There's such a thing as a rhetorical question,
and sometimes using that form is the most obvious & succinct way to
put things.

Which is not to say that phrasing these things as a non-question can't
be better, but the suggestions so far just seem more complex.

Also keep in mind that a huge part of the user base for git using the
English UI consists of non-native speakers, and when in doubt we
should definitely be picking simpler English like "did you mean?" v.s.
alternatives with >10 character more obscure words.

> Richard Kerry
> BNCS Engineer, SI SOL Telco & Media Vertical Practice
>
> T: +44 (0)20 3618 2669
> M: +44 (0)7812 325518
> Lync: +44 (0) 20 3618 0778
> Room G300, Stadium House, Wood Lane, London, W12 7TA
> richard.ke...@atos.net
>
>
>
>
> -Original Message-
> From: git-ow...@vger.kernel.org [mailto:git-ow...@vger.kernel.org] On Behalf 
> Of Jean-Noel Avila
> Sent: Wednesday, May 03, 2017 10:07 PM
> To: git@vger.kernel.org
> Cc: rashmipa...@gmail.com; Jean-Noel Avila 
> Subject: [PATCH v2 1/3] usability: don't ask questions if no reply is required
>
> There has been a bug report by a corporate user that stated that "spelling 
> mistake of stash followed by a yes prints character 'y'
> infinite times."
>
> This analysis was false. When the spelling of a command contains errors, the 
> git program tries to help the user by providing candidates which are close to 
> the unexisting command. E.g Git prints the
> following:
>
> git: 'stahs' is not a git command. See 'git --help'.
> Did you mean this?
>
> stash
>
> and then exits.
>
> The problem with this hint is that it is not formally indicated as an hint 
> and the user is in fact encouraged to reply to the question, whereas the Git 
> command is already finished.
>
> The user was unlucky enough that it was the command he was looking for, and 
> replied "yes" on the command line, effectively launching the `yes` program.
>
> The initial error is that the Git programs, when launched in command-line 
> mode (without interaction) must not ask questions, because thes

RE: [PATCH v2 1/3] usability: don't ask questions if no reply is required

[Kerry, Richard]

May I suggest that " The most approaching commands" doesn't make much sense as 
English (I don't think a command can "approach").
Perhaps it should be " The most appropriate commands".


Regards,
Richard.





Richard Kerry
BNCS Engineer, SI SOL Telco & Media Vertical Practice

T: +44 (0)20 3618 2669
M: +44 (0)7812 325518
Lync: +44 (0) 20 3618 0778
Room G300, Stadium House, Wood Lane, London, W12 7TA
richard.ke...@atos.net




-Original Message-
From: git-ow...@vger.kernel.org [mailto:git-ow...@vger.kernel.org] On Behalf Of 
Jean-Noel Avila
Sent: Wednesday, May 03, 2017 10:07 PM
To: git@vger.kernel.org
Cc: rashmipa...@gmail.com; Jean-Noel Avila 
Subject: [PATCH v2 1/3] usability: don't ask questions if no reply is required

There has been a bug report by a corporate user that stated that "spelling 
mistake of stash followed by a yes prints character 'y'
infinite times."

This analysis was false. When the spelling of a command contains errors, the 
git program tries to help the user by providing candidates which are close to 
the unexisting command. E.g Git prints the
following:

git: 'stahs' is not a git command. See 'git --help'.
Did you mean this?

stash

and then exits.

The problem with this hint is that it is not formally indicated as an hint and 
the user is in fact encouraged to reply to the question, whereas the Git 
command is already finished.

The user was unlucky enough that it was the command he was looking for, and 
replied "yes" on the command line, effectively launching the `yes` program.

The initial error is that the Git programs, when launched in command-line mode 
(without interaction) must not ask questions, because these questions would 
normally require a user input as a reply while they won't handle indeed. That's 
a source of confusion on UX level.

To improve the general usability of the Git suite, the following rule was 
applied:

if the sentence
 * appears in a non-interactive session
 * is printed last before exit
 * is a question addressing the user ("you")

the sentence is turned into affirmative and proposes the option.

The basic rewording of the question sentences has been extended to other spots 
found in the source.

Requested at https://github.com/git/git-scm.com/issues/999 by rpai1

Signed-off-by: Jean-Noel Avila 
---
 builtin/am.c   | 4 ++--
 builtin/checkout.c | 2 +-
 help.c | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c index a95dd8b4e..f5afa438d 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1312,7 +1312,7 @@ static int parse_mail(struct am_state *state, const char 
*mail)
}

if (is_empty_file(am_path(state, "patch"))) {
-   printf_ln(_("Patch is empty. Was it split wrong?"));
+   printf_ln(_("Patch is empty. It may have been split wrong."));
die_user_resolve(state);
}

@@ -1940,7 +1940,7 @@ static void am_resolve(struct am_state *state)

if (unmerged_cache()) {
printf_ln(_("You still have unmerged paths in your index.\n"
-   "Did you forget to use 'git add'?"));
+   "You might want to use 'git add' on them."));
die_user_resolve(state);
}

diff --git a/builtin/checkout.c b/builtin/checkout.c index bfa5419f3..05037b9b6 
100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1287,7 +1287,7 @@ int cmd_checkout(int argc, const char **argv, const char 
*prefix)
 */
if (opts.new_branch && argc == 1)
die(_("Cannot update paths and switch to branch '%s' at 
the same time.\n"
- "Did you intend to checkout '%s' which can not be 
resolved as commit?"),
+ "'%s' can not be resolved as commit, but it 
should."),
opts.new_branch, argv[0]);

if (opts.force_detach)
diff --git a/help.c b/help.c
index bc6cd19cf..4658a55c6 100644
--- a/help.c
+++ b/help.c
@@ -411,8 +411,8 @@ const char *help_unknown_cmd(const char *cmd)

if (SIMILAR_ENOUGH(best_similarity)) {
fprintf_ln(stderr,
-  Q_("\nDid you mean this?",
- "\nDid you mean one of these?",
+  Q_("\nThe most approaching command is",
+ "\nThe most approaching commands are",
   n));

for (i = 0; i < n; i++)
--
2.12.0

Atos, Atos Consulting, Worldline and Canopy The Open Cloud Company are trading 
names used by the Atos group. The following trading entities are registered in 
England and Wales: Atos IT Services UK Limited (registered number 01245534), 
Atos Consulting Limited (registered number 04312380), Atos Worldline UK Limited 
(registered number 08514184) and Canopy The Open Cloud Company Limited 
(registration number 08011902). The registered office for ea