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

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

[Jean-Noel Avila]

As described in the bug report at

https://github.com/git/git-scm.com/issues/999

the user was disconcerted by the question asked by the program not
requiring a reply from the user. 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.

Signed-off-by: Jean-Noel Avila <jn.avila@free.fr>
---
 help.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

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

[PATCH 2/4] usability: fix am and checkout for nevermind questions

[Jean-Noel Avila]

Signed-off-by: Jean-Noel Avila <jn.avila@free.fr>
---
 builtin/am.c       | 4 ++--
 builtin/checkout.c | 2 +-
 2 files changed, 3 insertions(+), 3 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)
-- 
2.12.0

[PATCH 3/4] read-tree.c: rework UI when merging no trees

[Jean-Noel Avila]

The initial test was inherited from a previous commit, but it is no
longer needed, given the following switch case. Moreover, the question
sentence ending the program has been replace by an assertative one.

Signed-off-by: Jean-Noel Avila <jn.avila@free.fr>
---
 builtin/read-tree.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/builtin/read-tree.c b/builtin/read-tree.c
index 23e212ee8..05296997c 100644
--- a/builtin/read-tree.c
+++ b/builtin/read-tree.c
@@ -226,9 +226,10 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
 		setup_work_tree();
 
 	if (opts.merge) {
-		if (stage < 2)
-			die("just how do you expect me to merge %d trees?", stage-1);
 		switch (stage - 1) {
+		case 0:
+			die("there are no trees to merge!");
+			break;
 		case 1:
 			opts.fn = opts.prefix ? bind_merge : oneway_merge;
 			break;
-- 
2.12.0

[PATCH 4/4] git-filter-branch: be assertative on dying message

[Jean-Noel Avila]

Signed-off-by: Jean-Noel Avila <jn.avila@free.fr>
---
 git-filter-branch.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 2b8cdba15..dd3a605d0 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -239,7 +239,7 @@ git rev-parse --no-flags --revs-only --symbolic-full-name \
 sed -e '/^^/d' "$tempdir"/raw-heads >"$tempdir"/heads
 
 test -s "$tempdir"/heads ||
-	die "Which ref do you want to rewrite?"
+	die "You must specify a ref to rewrite"
 
 GIT_INDEX_FILE="$(pwd)/../index"
 export GIT_INDEX_FILE
-- 
2.12.0

Re: [PATCH 1/4] usability: don't ask questions if no reply is required

[Jonathan Nieder]

Hi,

Jean-Noel Avila wrote:

> As described in the bug report at
>
> https://github.com/git/git-scm.com/issues/999

External issue tracker URLs have been known to change or disappear and
we try to make commit messages self-contained instead of relying on
them.  It is common to put a 'Requested-by:' footer or sentence saying
'Requested at <url> by <person>' near the bottom of a commit message
for attribution and context.  Relying on the bug report more heavily
like this example (instead of including any relevant information)
makes it harder for a reader to understand the patch easily in
one place.

In other words, instead of asking the reader to read the bug report,
please include pertinent information the reader needs to
understand the patch here so they don't have to.

> the user was disconcerted by the question asked by the program not
> requiring a reply from the user. 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.
>
> Signed-off-by: Jean-Noel Avila <jn.avila@free.fr>
> ---
>  help.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> 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 what it's worth, I find the new text harder to understand than the
old text.

From the bug report:

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

	stash

	Git asked if i meant git stash. and i entered yes. and git
	printed the character y infinite times.

If I'm reading that correctly, the problem is not that questions are
alarming but that Git did not cope well with the answer.  When I try
to reproduce it, I get

	$ git stahs
	WARNING: You called a Git command named 'stahs', which does not exist.
	Continuing under the assumption that you meant 'stash'
	in 5.0 seconds automatically...

which is much clearer.  After commenting out "[help] autocorrect = 50" in my
~/.config/git/config, I get

	$ git stahs
	git: 'stahs' is not a git command. See 'git --help'.

	Did you mean this?
		stash

which does seem improvable, at least for consistency with the
autocorrect case.  For example, would something like

	$ git stahs
	fatal: You called a Git command named 'stahs', which does not exist.
	hint: Did you mean 'git stash'?

work better?  And the autocorrect case could say something like

	$ git stahs
	warning: You called a Git command named 'stahs', which does not exist.
	warning: Continuing under the assumption that you meant 'stash'
	warning: in 5.0 seconds automatically...

Is contact information for the bug reporter available so we can try out
different wordings and see what works for them?

Thanks and hope that helps,
Jonathan

Re: [PATCH 2/4] usability: fix am and checkout for nevermind questions

[Jonathan Nieder]

Jean-Noel Avila wrote:

> Subject: usability: fix am and checkout for nevermind questions
>
> Signed-off-by: Jean-Noel Avila <jn.avila@free.fr>

Thanks for working on improving Git's UX.  I agree with the goal in
general (we should not gratuitously surprise users) but I think I
lack context for appreciating this particular example.

This is a good place to describe the motivation behind the patch and
what effective change it would have.

[...]
> +++ b/builtin/am.c
[...]
>  	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."));
[...]
>  	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."));
[...]
>  		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."),

In the current state I think this patch makes things worse (questions
are not automatically a bad thing), which would make it especially
useful to see more about the motivation so we can find out whether
there's another way.

Thanks,
Jonathan

Re: [PATCH 1/4] usability: don't ask questions if no reply is required

[Stefan Beller]

+cc rashmipai36@gmail.com

On Wed, May 3, 2017 at 9:47 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Hi,
>
> Jean-Noel Avila wrote:
>
>> As described in the bug report at
>>
>> https://github.com/git/git-scm.com/issues/999
>
> External issue tracker URLs have been known to change or disappear and
> we try to make commit messages self-contained instead of relying on
> them.  It is common to put a 'Requested-by:' footer or sentence saying
> 'Requested at <url> by <person>' near the bottom of a commit message
> for attribution and context.  Relying on the bug report more heavily
> like this example (instead of including any relevant information)
> makes it harder for a reader to understand the patch easily in
> one place.
>
> In other words, instead of asking the reader to read the bug report,
> please include pertinent information the reader needs to
> understand the patch here so they don't have to.
>
>> the user was disconcerted by the question asked by the program not
>> requiring a reply from the user. 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.
>>
>> Signed-off-by: Jean-Noel Avila <jn.avila@free.fr>
>> ---
>>  help.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> 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 what it's worth, I find the new text harder to understand than the
> old text.
>
> From the bug report:
>
>         Now git says git: 'stahs' is not a git command. See 'git --help'.
>         Did you mean this?
>
>         stash
>
>         Git asked if i meant git stash. and i entered yes. and git
>         printed the character y infinite times.
>
> If I'm reading that correctly, the problem is not that questions are
> alarming but that Git did not cope well with the answer.  When I try
> to reproduce it, I get
>
>         $ git stahs
>         WARNING: You called a Git command named 'stahs', which does not exist.
>         Continuing under the assumption that you meant 'stash'
>         in 5.0 seconds automatically...
>
> which is much clearer.  After commenting out "[help] autocorrect = 50" in my
> ~/.config/git/config, I get
>
>         $ git stahs
>         git: 'stahs' is not a git command. See 'git --help'.
>
>         Did you mean this?
>                 stash
>
> which does seem improvable, at least for consistency with the
> autocorrect case.  For example, would something like
>
>         $ git stahs
>         fatal: You called a Git command named 'stahs', which does not exist.
>         hint: Did you mean 'git stash'?
>
> work better?  And the autocorrect case could say something like
>
>         $ git stahs
>         warning: You called a Git command named 'stahs', which does not exist.
>         warning: Continuing under the assumption that you meant 'stash'
>         warning: in 5.0 seconds automatically...
>
> Is contact information for the bug reporter available so we can try out
> different wordings and see what works for them?

yes, cc'd.
Also see
https://public-inbox.org/git/CAOqCAXSOZCG8mijV+yATtmC1PFGYiOSqtraSdbhbP2rRHBO_Qg@mail.gmail.com

>
> Thanks and hope that helps,
> Jonathan

Re: [PATCH 3/4] read-tree.c: rework UI when merging no trees

[Jonathan Nieder]

Hi,

Jean-Noel Avila wrote:

> Subject: read-tree.c: rework UI when merging no trees

nit: this is about user-facing behavior, not an implementation detail,
so the part before the colon can be the command that changed
(read-tree:).

nit: the word "rework" is dangerous in a commit message in the same
way as the word "fix" --- it stands for "make better", in a vague way
that leaves the reader guessing about how.  Usually a more specific
description can work better.

> The initial test was inherited from a previous commit, but it is no
> longer needed, given the following switch case. Moreover, the question
> sentence ending the program has been replace by an assertative one.
> 
> Signed-off-by: Jean-Noel Avila <jn.avila@free.fr>

This can have a simpler, short-and-sweet motivation:

	read-tree -m: make error message for merging 0 trees less smart-alecky

	"git read-tree -m" requires a tree argument to name the tree to be
	merged in.  Git uses a cutesy error message to say so and why:

		$ git read-tree -m
		warning: read-tree: emptying the index with no arguments is deprecated; use --empty
		fatal: just how do you expect me to merge 0 trees?
		$ git read-tree -m --empty
		fatal: just how do you expect me to merge 0 trees?

	When lucky, that could produce an ah-hah moment for the user, but it's
	more likely to irritate and distract them.

	Instead, tell the user plainly that the tree argument is required. Also
	document this requirement in the git-read-tree(1) manpage where there is
	room to explain it in a more straightforward way.

Unfortunately both 'git read-tree -h' and 'git read-tree --help' say nothing about
this.  Ideas for wording there?

Thanks and hope that helps,
Jonathan

Re: [PATCH 4/4] git-filter-branch: be assertative on dying message

[Jonathan Nieder]

Hi,

Jean-Noel Avila wrote:

> Signed-off-by: Jean-Noel Avila <jn.avila@free.fr>

As with the previous patches, this is a good place to put the motivation
for the patch.

> ---
>  git-filter-branch.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/git-filter-branch.sh b/git-filter-branch.sh
> index 2b8cdba15..dd3a605d0 100755
> --- a/git-filter-branch.sh
> +++ b/git-filter-branch.sh
> @@ -239,7 +239,7 @@ git rev-parse --no-flags --revs-only --symbolic-full-name \
>  sed -e '/^^/d' "$tempdir"/raw-heads >"$tempdir"/heads
>  
>  test -s "$tempdir"/heads ||
> -	die "Which ref do you want to rewrite?"
> +	die "You must specify a ref to rewrite"

I find both the old and the new messages pretty uncompelling.  The user
got the usage wrong but we don't know what they were trying to do ---
e.g. maybe they specified the ref to rewrite but in the wrong place.

Would e.g. a simple call to 'usage' work?

Thanks,
Jonathan

Re: [PATCH 1/4] usability: don't ask questions if no reply is required

[Jean-Noël AVILA]

Le mercredi 3 mai 2017, 09:47:44 CEST Jonathan Nieder a écrit :
> Hi,
> 
> Jean-Noel Avila wrote:
> > As described in the bug report at
> > 
> > https://github.com/git/git-scm.com/issues/999
> 
> External issue tracker URLs have been known to change or disappear and
> we try to make commit messages self-contained instead of relying on
> them.  It is common to put a 'Requested-by:' footer or sentence saying
> 'Requested at <url> by <person>' near the bottom of a commit message
> for attribution and context.  Relying on the bug report more heavily
> like this example (instead of including any relevant information)
> makes it harder for a reader to understand the patch easily in
> one place.
> 
> In other words, instead of asking the reader to read the bug report,
> please include pertinent information the reader needs to
> understand the patch here so they don't have to.

Ok. Will include more context in the commit message and just provide the BT as 
an additional link.

> 
> > the user was disconcerted by the question asked by the program not
> > requiring a reply from the user. 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.
> > 
> > Signed-off-by: Jean-Noel Avila <jn.avila@free.fr>
> > ---
> > 
> >  help.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > 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 what it's worth, I find the new text harder to understand than the
> old text.
> 
> From the bug report:
> 
> 	Now git says git: 'stahs' is not a git command. See 'git --help'.
> 	Did you mean this?
> 
> 	stash
> 
> 	Git asked if i meant git stash. and i entered yes. and git
> 	printed the character y infinite times.
> 
> If I'm reading that correctly, the problem is not that questions are
> alarming but that Git did not cope well with the answer.  When I try
> to reproduce it, I get

No, I don't think that the questions are alarming. The whole point is that Git 
no longer runs when the user enters its reply. In the case of the bug report, 
the user was unlucky to type in the name of the shell command `yes` because he 
was thinking that Git was still running interactively, due to the question at 
the end of the run.

So this patch series'aim is simply to get rid of asking questions just before 
exiting. Even if a question might seem more user friendly, it's insufficiently 
formal to indicate to the user that there's no point replying. The question 
was just a hint, and it should presented as such.

To be fair, I'm not accustomed enough to the code to know exactly in which 
cases the given strings are occurring (except here). All the patch series 
tries to tackle this at different levels. Maybe squashing them all would be 
better for understanding. 

> 
> 	$ git stahs
> 	WARNING: You called a Git command named 'stahs', which does not exist.
> 	Continuing under the assumption that you meant 'stash'
> 	in 5.0 seconds automatically...
> 
> which is much clearer.  After commenting out "[help] autocorrect = 50" in my
> ~/.config/git/config, I get
> 
> 	$ git stahs
> 	git: 'stahs' is not a git command. See 'git --help'.
> 
> 	Did you mean this?
> 		stash
> 
> which does seem improvable, at least for consistency with the
> autocorrect case.  For example, would something like
> 
> 	$ git stahs
> 	fatal: You called a Git command named 'stahs', which does not exist.
> 	hint: Did you mean 'git stash'?
> 
> work better?  And the autocorrect case could say something like

Would adding a "hint:" prefix be enough to provide context? I don't think so. 
I'd prefer to be clearer on the objectives of the printed information, even at 
the risk of being clumsy.


>
> 	$ git stahs
> 	warning: You called a Git command named 'stahs', which does not exist.
> 	warning: Continuing under the assumption that you meant 'stash'
> 	warning: in 5.0 seconds automatically...
> 
> Is contact information for the bug reporter available so we can try out
> different wordings and see what works for them?

I guess so. The discussion on github is still open and only depends on the 
willingness of the reporter to reply.


> 
> Thanks and hope that helps,
> Jonathan

Re: [PATCH 2/4] usability: fix am and checkout for nevermind questions

[Jean-Noël AVILA]

Le mercredi 3 mai 2017 09:51:58 CEST, vous avez écrit :
> Jean-Noel Avila wrote:
> > Subject: usability: fix am and checkout for nevermind questions
> > 
> > Signed-off-by: Jean-Noel Avila <jn.avila@free.fr>
> 
> Thanks for working on improving Git's UX.  I agree with the goal in
> general (we should not gratuitously surprise users) but I think I
> lack context for appreciating this particular example.
> 
> This is a good place to describe the motivation behind the patch and
> what effective change it would have.
> 
> [...]
> 
> > +++ b/builtin/am.c
> 
> [...]
> 
> >  	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."));
> 
> [...]
> 
> >  	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."));
> 
> [...]
> 
> >  		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."),
> 
> In the current state I think this patch makes things worse (questions
> are not automatically a bad thing), which would make it especially
> useful to see more about the motivation so we can find out whether
> there's another way.
> 

I am not a UX designer, but for me, in the context of interaction with a 
command line program, any question that does not accept a reply is bad design. 
That also means that any command that does not run interactively should not 
ask questions. The shell interface is too informal to allow being loose on the 
program side. Comparatively to a GUI, where a label is formally informative 
and a popping-up dialog box asks for user input.

This patch should indeed be squashed with the first one. They are small changes 
in strings printed when dying.  They would share the more extended commit 
message.

Re: [PATCH 3/4] read-tree.c: rework UI when merging no trees

[Jean-Noël AVILA]

Le mercredi 3 mai 2017, 10:04:01 CEST Jonathan Nieder a écrit :
> Hi,
> 
> Jean-Noel Avila wrote:
> > Subject: read-tree.c: rework UI when merging no trees
> 
> nit: this is about user-facing behavior, not an implementation detail,
> so the part before the colon can be the command that changed
> (read-tree:).
> 
> nit: the word "rework" is dangerous in a commit message in the same
> way as the word "fix" --- it stands for "make better", in a vague way
> that leaves the reader guessing about how.  Usually a more specific
> description can work better.
> 

In fact, this patch is two fold:

 * reword the question in the die() call. I realize now that when passed to 
die(), the string is prepended with "fatal:". That's an hint that the question 
does not require a reply, but  ruling out any doubt would be better.
 * rework the local logic which was inherited from history. This is 
functionally equivalent to the previous version, just cleaner.

> > The initial test was inherited from a previous commit, but it is no
> > longer needed, given the following switch case. Moreover, the question
> > sentence ending the program has been replace by an assertative one.
> > 
> > Signed-off-by: Jean-Noel Avila <jn.avila@free.fr>
> 
> This can have a simpler, short-and-sweet motivation:
> 
> 	read-tree -m: make error message for merging 0 trees less smart-alecky
> 
> 	"git read-tree -m" requires a tree argument to name the tree to be
> 	merged in.  Git uses a cutesy error message to say so and why:
> 
> 		$ git read-tree -m
> 		warning: read-tree: emptying the index with no arguments is deprecated;
> use --empty fatal: just how do you expect me to merge 0 trees?
> 		$ git read-tree -m --empty
> 		fatal: just how do you expect me to merge 0 trees?
> 
> 	When lucky, that could produce an ah-hah moment for the user, but it's
> 	more likely to irritate and distract them.
> 
> 	Instead, tell the user plainly that the tree argument is required. Also
> 	document this requirement in the git-read-tree(1) manpage where there is
> 	room to explain it in a more straightforward way.
> 

Thank you very much for this message! May I s-o-b you?

As hinted, I'll add the documentation part. ;-)

> Unfortunately both 'git read-tree -h' and 'git read-tree --help' say nothing
> about this.  Ideas for wording there?

Next pach series will propose this.

> 
> Thanks and hope that helps,
> Jonathan

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

[Jean-Noel Avila]

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 <jn.avila@free.fr>
---
 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

[PATCH v2 2/3] read-tree -m: make error message for merging 0 trees less smart aleck

[Jean-Noel Avila]

"git read-tree -m" requires a tree argument to name the tree to be
merged in.  Git uses a cutesy error message to say so and why:

    $ git read-tree -m
    warning: read-tree: emptying the index with no arguments is
    deprecated; use --empty
    fatal: just how do you expect me to merge 0 trees?
    $ git read-tree -m --empty
    fatal: just how do you expect me to merge 0 trees?

When lucky, that could produce an ah-hah moment for the user, but it's
more likely to irritate and distract them.

Instead, tell the user plainly that the tree argument is
required. Also document this requirement in the git-read-tree(1)
manpage where there is room to explain it in a more straightforward way.

Signed-off-by: Jean-Noel Avila <jn.avila@free.fr>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 Documentation/git-read-tree.txt | 8 ++++----
 builtin/read-tree.c             | 7 ++++---
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-read-tree.txt b/Documentation/git-read-tree.txt
index ed9d63ef4..7cd9c6306 100644
--- a/Documentation/git-read-tree.txt
+++ b/Documentation/git-read-tree.txt
@@ -135,10 +135,10 @@ OPTIONS
 
 Merging
 -------
-If `-m` is specified, 'git read-tree' can perform 3 kinds of
-merge, a single tree merge if only 1 tree is given, a
-fast-forward merge with 2 trees, or a 3-way merge if 3 trees are
-provided.
+If `-m` is specified, at least one tree must be given on the command
+line. 'git read-tree' can perform 3 kinds of merge, a single tree
+merge if only 1 tree is given, a fast-forward merge with 2 trees, or a
+3-way merge if 3 trees are provided.
 
 
 Single Tree Merge
diff --git a/builtin/read-tree.c b/builtin/read-tree.c
index 23e212ee8..68c5b0ca4 100644
--- a/builtin/read-tree.c
+++ b/builtin/read-tree.c
@@ -132,7 +132,7 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
 		OPT_BOOL(0, "empty", &read_empty,
 			    N_("only empty the index")),
 		OPT__VERBOSE(&opts.verbose_update, N_("be verbose")),
-		OPT_GROUP(N_("Merging")),
+		OPT_GROUP(N_("Merging (needs at least one tree-ish")),
 		OPT_BOOL('m', NULL, &opts.merge,
 			 N_("perform a merge in addition to a read")),
 		OPT_BOOL(0, "trivial", &opts.trivial_merges_only,
@@ -226,9 +226,10 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
 		setup_work_tree();
 
 	if (opts.merge) {
-		if (stage < 2)
-			die("just how do you expect me to merge %d trees?", stage-1);
 		switch (stage - 1) {
+		case 0:
+			die("you must specify at least one tree to merge");
+			break;
 		case 1:
 			opts.fn = opts.prefix ? bind_merge : oneway_merge;
 			break;
-- 
2.12.0

[PATCH v2 3/3] git-filter-branch: make the error msg when missing branch more open

[Jean-Noel Avila]

git-filter-branch requires the specification of a branch by one way or
another. If no branch appears to have been specified, we know the user
got the usage wrong but we don't know what they were trying to do ---
e.g. maybe they specified the ref to rewrite but in the wrong place.

The safest solution is to just print the usage in this case.

Signed-off-by: Jean-Noel Avila <jn.avila@free.fr>
---
 git-filter-branch.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 2b8cdba15..bda2bae23 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -239,7 +239,7 @@ git rev-parse --no-flags --revs-only --symbolic-full-name \
 sed -e '/^^/d' "$tempdir"/raw-heads >"$tempdir"/heads
 
 test -s "$tempdir"/heads ||
-	die "Which ref do you want to rewrite?"
+	usage
 
 GIT_INDEX_FILE="$(pwd)/../index"
 export GIT_INDEX_FILE
-- 
2.12.0

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.kerry@atos.net




-----Original Message-----
From: git-owner@vger.kernel.org [mailto:git-owner@vger.kernel.org] On Behalf Of Jean-Noel Avila
Sent: Wednesday, May 03, 2017 10:07 PM
To: git@vger.kernel.org
Cc: rashmipai36@gmail.com; Jean-Noel Avila <jn.avila@free.fr>
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 <jn.avila@free.fr>
---
 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 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

[Ævar Arnfjörð Bjarmason]

On Thu, May 4, 2017 at 10:52 AM, Kerry, Richard <richard.kerry@atos.net> 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.kerry@atos.net
>
>
>
>
> -----Original Message-----
> From: git-owner@vger.kernel.org [mailto:git-owner@vger.kernel.org] On Behalf Of Jean-Noel Avila
> Sent: Wednesday, May 03, 2017 10:07 PM
> To: git@vger.kernel.org
> Cc: rashmipai36@gmail.com; Jean-Noel Avila <jn.avila@free.fr>
> 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 <jn.avila@free.fr>
> ---
>  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 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

[Jean-Noël AVILA]

Le jeudi 4 mai 2017, 08:52:43 CEST Kerry, Richard a écrit :
> 
> 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.
> 



Thank you for your proposition. "approaching" is a frenchism  doubled with google translate (sorry!). Maybe "similar" would also work.
  

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.kerry@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 [avarab@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 <richard.kerry@atos.net> 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.kerry@atos.net
>
>
>
>
> -----Original Message-----
> From: git-owner@vger.kernel.org [mailto:git-owner@vger.kernel.org] On Behalf Of Jean-Noel Avila
> Sent: Wednesday, May 03, 2017 10:07 PM
> To: git@vger.kernel.org
> Cc: rashmipai36@gmail.com; Jean-Noel Avila <jn.avila@free.fr>
> 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 <jn.avila@free.fr>
> ---
>  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 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.
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

[Jean-Noël AVILA]

Le jeudi 4 mai 2017, 10:14:58 CEST Kerry, Richard a écrit :
> 
> 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.
> 

Thanks. So what's the status of this patch series? I don't buy the idea of rhetorical HMI. That's a sure way to confuse non-native speakers. Please note that I kept the questions when there is a following text. Only questions addressing the user at the end of output have been rephrased.

For the "do you mean" questions, the proposition would then simply be: "the most similar command is:" or "the most similar commands are:".

and then  what about the other patches?

Thanks


> 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.kerry@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 [avarab@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 <richard.kerry@atos.net> 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.kerry@atos.net
> >
> >
> >
> >
> > -----Original Message-----
> > From: git-owner@vger.kernel.org [mailto:git-owner@vger.kernel.org] On Behalf Of Jean-Noel Avila
> > Sent: Wednesday, May 03, 2017 10:07 PM
> > To: git@vger.kernel.org
> > Cc: rashmipai36@gmail.com; Jean-Noel Avila <jn.avila@free.fr>
> > 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 <jn.avila@free.fr>
> > ---
> >  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 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.
> 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

[Ævar Arnfjörð Bjarmason]

On Tue, May 9, 2017 at 10:18 AM, Jean-Noël AVILA <jn.avila@free.fr> wrote:
> Le jeudi 4 mai 2017, 10:14:58 CEST Kerry, Richard a écrit :
>>
>> 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.
>>
>
> Thanks. So what's the status of this patch series? I don't buy the idea of rhetorical HMI. That's a sure way to confuse non-native speakers. Please note that I kept the questions when there is a following text. Only questions addressing the user at the end of output have been rephrased.
>
> For the "do you mean" questions, the proposition would then simply be: "the most similar command is:" or "the most similar commands are:".
>
> and then  what about the other patches?

When you submit patches you can monitor the next "What's cooking" mail
for the status. See "ja/do-not..." here:
https://public-inbox.org/git/xmqqlgq77pse.fsf@gitster.mtv.corp.google.com/

It got picked up for the "pu" branch. You can fetch git.git and see it there.

My feedback on the 3:

* 1/3: Mostly covered above. I did notice after my last comment that
every time gcc wants to suggest you should do something different
(e.g. misspelled variable or macro) it'll say "did you mean?" similar
to what git does now.

While I think this is a rather tragic story of *nix usability ("user
gets asked a question, types yes, gets a few GB/s of y as output") the
main UX problem is surely that the user in question didn't understand
from the terminal output when the program had exited & wasn't
interactive anymore.

But overall this seems like optimizing for a really obscure edge case
at the expense of making the wording more clever. I don't think "did
you mean?" will confuse non-native speakers, as the bug report shows
the user in question has a reasonable command of English, they're
fundimentally confused about how the shell interface works.

* 2/3: Looks great, surprised it took so long for someone to remove
that cutsey but bad message.

* 3/3: I think this partly makes things slightly worse. I.e. now you
get a specific error message about refs being missing, after it shows
you the entire usage info, so you don't know if you e.g. misspelled a
command-line flag or what. I couldn't find any pattern in the existing
shell scripts for "print usage with custom message" thoug.

>> 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.kerry@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 [avarab@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 <richard.kerry@atos.net> 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.kerry@atos.net
>> >
>> >
>> >
>> >
>> > -----Original Message-----
>> > From: git-owner@vger.kernel.org [mailto:git-owner@vger.kernel.org] On Behalf Of Jean-Noel Avila
>> > Sent: Wednesday, May 03, 2017 10:07 PM
>> > To: git@vger.kernel.org
>> > Cc: rashmipai36@gmail.com; Jean-Noel Avila <jn.avila@free.fr>
>> > 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 <jn.avila@free.fr>
>> > ---
>> >  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 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.
>> 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

[Junio C Hamano]

Jean-Noel Avila <jn.avila@free.fr> 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.

> @@ -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.

> 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);

> 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
this may be an improvement.

Thanks.

Re: [PATCH v2 2/3] read-tree -m: make error message for merging 0 trees less smart aleck

[Junio C Hamano]

Jean-Noel Avila <jn.avila@free.fr> writes:

> "git read-tree -m" requires a tree argument to name the tree to be
> merged in.  Git uses a cutesy error message to say so and why:
>
>     $ git read-tree -m
>     warning: read-tree: emptying the index with no arguments is
>     deprecated; use --empty
>     fatal: just how do you expect me to merge 0 trees?
>     $ git read-tree -m --empty
>     fatal: just how do you expect me to merge 0 trees?

This shows another issue.  The "emptying ... is deprecated" message
shouldn't be given when -m is present.

I am not saying that that needs to be fixed by you and/or as a part
of this patch.  Just something I noticed while reviewing the patch.

>  Merging
>  -------
> -If `-m` is specified, 'git read-tree' can perform 3 kinds of
> -merge, a single tree merge if only 1 tree is given, a
> -fast-forward merge with 2 trees, or a 3-way merge if 3 trees are
> -provided.
> +If `-m` is specified, at least one tree must be given on the command
> +line. 'git read-tree' can perform 3 kinds of merge, a single tree
> +merge if only 1 tree is given, a fast-forward merge with 2 trees, or a
> +3-way merge if 3 trees are provided.

It may not incorrect per-se, but the existing enumeration already
say 1, 2 and 3 are the valid choices, so "at least one" may be
redundant.

One incorrectness that needs to be changed is "if 3 trees are
provided"; it is "if 3 or more trees".  Again, not the topic of your
change, but this one you may want to address while you are at it.

>  	if (opts.merge) {
> -		if (stage < 2)
> -			die("just how do you expect me to merge %d trees?", stage-1);
>  		switch (stage - 1) {
> +		case 0:

Could "stage" be 0 (or negative) when we come here?  If so, this rewrite
may no longer diagnose the error correctly in such a case.

	... goes and looks ...

I think it begins with either 0 or 1 and then only counts up, so we
should be safe.  Rolling it in the switch() like this patch does
makes it easier to follow what is going on, I think.

> +			die("you must specify at least one tree to merge");
> +			break;
>  		case 1:
>  			opts.fn = opts.prefix ? bind_merge : oneway_merge;
>  			break;

Thanks.

Re: [PATCH v2 3/3] git-filter-branch: make the error msg when missing branch more open

[Junio C Hamano]

Jean-Noel Avila <jn.avila@free.fr> writes:

> git-filter-branch requires the specification of a branch by one way or
> another. If no branch appears to have been specified, we know the user
> got the usage wrong but we don't know what they were trying to do ---
> e.g. maybe they specified the ref to rewrite but in the wrong place.
>
> The safest solution is to just print the usage in this case.
>
> Signed-off-by: Jean-Noel Avila <jn.avila@free.fr>
> ---
>  git-filter-branch.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/git-filter-branch.sh b/git-filter-branch.sh
> index 2b8cdba15..bda2bae23 100755
> --- a/git-filter-branch.sh
> +++ b/git-filter-branch.sh
> @@ -239,7 +239,7 @@ git rev-parse --no-flags --revs-only --symbolic-full-name \
>  sed -e '/^^/d' "$tempdir"/raw-heads >"$tempdir"/heads
>  
>  test -s "$tempdir"/heads ||
> -	die "Which ref do you want to rewrite?"
> +	usage
>  
>  GIT_INDEX_FILE="$(pwd)/../index"
>  export GIT_INDEX_FILE

I tend to agree with Ævar on this one.  It is not apparent to the
end user after this change what exactly was wrong in the input; for
that matter, it is not even clear that the command is refusing to
run because it found problem with the input.  

Trying to move away from asking "I didn't get that, what did you
mean?" is one thing, and that can be done by saying "no ref to
rewrite given" or something.  We may want to make it into a more
"positive" nudge, telling the user what to do, e.g. "give me the
refs to rewrite."

Thanks.

[PATCH] read-tree: "read-tree -m --empty" does not make sense

[Junio C Hamano]

fb1bb965 ("read-tree: deprecate syntax without tree-ish args",
2010-09-10) wanted to deprecate "git read-tree" without any tree,
which used to be the way to empty the index, and encourage use of
"git read-tree --empty" instead.  

However, when used with "-m", "--empty" does not make any sense,
either, simply because merging 0 trees will result in a different
error anyway.

Omit the deprecation warning and let the code to emit real error
message diagnose the error.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

    Junio C Hamano <gitster@pobox.com> writes:

    > Jean-Noel Avila <jn.avila@free.fr> writes:
    >
    >> "git read-tree -m" requires a tree argument to name the tree to be
    >> merged in.  Git uses a cutesy error message to say so and why:
    >>
    >>     $ git read-tree -m
    >>     warning: read-tree: emptying the index with no arguments is
    >>     deprecated; use --empty
    >>     fatal: just how do you expect me to merge 0 trees?
    >>     $ git read-tree -m --empty
    >>     fatal: just how do you expect me to merge 0 trees?
    >
    > This shows another issue.  The "emptying ... is deprecated" message
    > shouldn't be given when -m is present.
    >
    > I am not saying that that needs to be fixed by you and/or as a part
    > of this patch.  Just something I noticed while reviewing the patch.

 builtin/read-tree.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/read-tree.c b/builtin/read-tree.c
index 23e212ee8c..284de743c3 100644
--- a/builtin/read-tree.c
+++ b/builtin/read-tree.c
@@ -210,7 +210,7 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
 			die("failed to unpack tree object %s", arg);
 		stage++;
 	}
-	if (nr_trees == 0 && !read_empty)
+	if (!nr_trees && !read_empty && !opts.merge)
 		warning("read-tree: emptying the index with no arguments is deprecated; use --empty");
 	else if (nr_trees > 0 && read_empty)
 		die("passing trees as arguments contradicts --empty");
-- 
2.13.0-336-gf73534b083

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-owner@vger.kernel.org [mailto:git-owner@vger.kernel.org] On
> Behalf Of Junio C Hamano
> Sent: Thursday, May 11, 2017 4:16 AM
> To: Jean-Noel Avila <jn.avila@free.fr>
> Cc: git@vger.kernel.org; rashmipai36@gmail.com
> Subject: Re: [PATCH v2 1/3] usability: don't ask questions if no reply is
> required
>
> Jean-Noel Avila <jn.avila@free.fr> 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 this may be an
> improvement.
>
> Thanks.

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.kerry@atos.net


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

[Konstantin Khomoutov]

On Thu, May 11, 2017 at 10:10:05AM +0000, Kerry, Richard wrote:

[...]
> > > @@ -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.".

Why not just

  You should run `git add` on each file with resolved conflicts to mark
  them as such.

I'm not an English speaker but IMHO this phrasing concentrates on the
essense of the problem.  It's far from being succint, unfortunately.

I also wonder what to do with "deleted by them" state of certain files
which are also "unmerged" but `git add`-ing them would be a wrong thing
to do if we want to accept the upstream's decision to delete the file.
So maybe something like

  You might run `git rm` on a file to accept "deleted by them" for it.

appended to the original hint would be good.

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

[Ævar Arnfjörð Bjarmason]

On Thu, May 11, 2017 at 12:28 PM, Konstantin Khomoutov
<kostix+git@007spb.ru> wrote:
> On Thu, May 11, 2017 at 10:10:05AM +0000, Kerry, Richard wrote:
>
> [...]
>> > > @@ -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.".
>
> Why not just
>
>   You should run `git add` on each file with resolved conflicts to mark
>   them as such.
>
> I'm not an English speaker but IMHO this phrasing concentrates on the
> essense of the problem.  It's far from being succint, unfortunately.
>
> I also wonder what to do with "deleted by them" state of certain files
> which are also "unmerged" but `git add`-ing them would be a wrong thing
> to do if we want to accept the upstream's decision to delete the file.
> So maybe something like
>
>   You might run `git rm` on a file to accept "deleted by them" for it.
>
> appended to the original hint would be good.

I think something like this sounds much better. I think being a bit
more verbose is good here, if you know how to solve conflicts you just
go "oops, forgot", but for confused users who don't know how, it's
better to explain things a bit more verbosely.

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

[Jean-Noel Avila]

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
that 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 <jn.avila@free.fr>
---
 builtin/am.c       | 5 +++--
 builtin/checkout.c | 5 ++---
 help.c             | 4 ++--
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index a95dd8b4e..dd60fad1e 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."));
 		die_user_resolve(state);
 	}
 
@@ -1940,7 +1940,8 @@ 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 should 'git add' each file with resolved conflicts to mark them as such.\n"
+			"You might run `git rm` on a file to accept \"deleted by them\" for it."));
 		die_user_resolve(state);
 	}
 
diff --git a/builtin/checkout.c b/builtin/checkout.c
index bfa5419f3..85c04d252 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1286,9 +1286,8 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		 * new_branch && argc > 1 will be caught later.
 		 */
 		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?"),
-			    opts.new_branch, argv[0]);
+			die(_("'%s' is not a commit and a branch '%s' cannot be created from it"),
+				argv[0], opts.new_branch);
 
 		if (opts.force_detach)
 			die(_("git checkout: --detach does not take a path argument '%s'"),
diff --git a/help.c b/help.c
index bc6cd19cf..a07f01e6f 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 similar command is",
+			      "\nThe most similar commands are",
 			   n));
 
 		for (i = 0; i < n; i++)
-- 
2.13.0

[PATCH v3 2/3] read-tree -m: make error message for merging 0 trees less smart aleck

[Jean-Noel Avila]

"git read-tree -m" requires a tree argument to name the tree to be
merged in.  Git uses a cutesy error message to say so and why:

    $ git read-tree -m
    warning: read-tree: emptying the index with no arguments is
    deprecated; use --empty
    fatal: just how do you expect me to merge 0 trees?
    $ git read-tree -m --empty
    fatal: just how do you expect me to merge 0 trees?

When lucky, that could produce an ah-hah moment for the user, but it's
more likely to irritate and distract them.

Instead, tell the user plainly that the tree argument is
required. Also document this requirement in the git-read-tree(1)
manpage where there is room to explain it in a more straightforward way.

Signed-off-by: Jean-Noel Avila <jn.avila@free.fr>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 Documentation/git-read-tree.txt | 8 ++++----
 builtin/read-tree.c             | 7 ++++---
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-read-tree.txt b/Documentation/git-read-tree.txt
index ed9d63ef4..97df00043 100644
--- a/Documentation/git-read-tree.txt
+++ b/Documentation/git-read-tree.txt
@@ -135,10 +135,10 @@ OPTIONS
 
 Merging
 -------
-If `-m` is specified, 'git read-tree' can perform 3 kinds of
-merge, a single tree merge if only 1 tree is given, a
-fast-forward merge with 2 trees, or a 3-way merge if 3 trees are
-provided.
+If `-m` is specified, at least one tree must be given on the command
+line. 'git read-tree' can perform 3 kinds of merge, a single tree
+merge if only 1 tree is given, a fast-forward merge with 2 trees, or a
+3-way merge if 3 or more trees are provided.
 
 
 Single Tree Merge
diff --git a/builtin/read-tree.c b/builtin/read-tree.c
index 23e212ee8..de1a58d17 100644
--- a/builtin/read-tree.c
+++ b/builtin/read-tree.c
@@ -132,7 +132,7 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
 		OPT_BOOL(0, "empty", &read_empty,
 			    N_("only empty the index")),
 		OPT__VERBOSE(&opts.verbose_update, N_("be verbose")),
-		OPT_GROUP(N_("Merging")),
+		OPT_GROUP(N_("Merging (needs at least one tree-ish")),
 		OPT_BOOL('m', NULL, &opts.merge,
 			 N_("perform a merge in addition to a read")),
 		OPT_BOOL(0, "trivial", &opts.trivial_merges_only,
@@ -226,9 +226,10 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
 		setup_work_tree();
 
 	if (opts.merge) {
-		if (stage < 2)
-			die("just how do you expect me to merge %d trees?", stage-1);
 		switch (stage - 1) {
+		case 0:
+			die(_("you must specify at least one tree to merge"));
+			break;
 		case 1:
 			opts.fn = opts.prefix ? bind_merge : oneway_merge;
 			break;
-- 
2.13.0

[PATCH v3 3/3] git-filter-branch:

[Jean-Noel Avila]

git-filter-branch requires the specification of a branch by one way or
another. If no branch appears to have been specified, we know the user
got the usage wrong but we don't know what they were trying to do ---
e.g. maybe they specified the ref to rewrite but in the wrong place.

In this case, just state that the branch specification is missing.

Signed-off-by: Jean-Noel Avila <jn.avila@free.fr>
---
 git-filter-branch.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 2b8cdba15..aafaf708d 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -239,7 +239,7 @@ git rev-parse --no-flags --revs-only --symbolic-full-name \
 sed -e '/^^/d' "$tempdir"/raw-heads >"$tempdir"/heads
 
 test -s "$tempdir"/heads ||
-	die "Which ref do you want to rewrite?"
+	die "You must specify a ref to rewrite."
 
 GIT_INDEX_FILE="$(pwd)/../index"
 export GIT_INDEX_FILE
-- 
2.13.0

Re: [PATCH v3 2/3] read-tree -m: make error message for merging 0 trees less smart aleck

[Jonathan Nieder]

Hi,

Jean-Noel Avila wrote:

> Signed-off-by: Jean-Noel Avila <jn.avila@free.fr>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>

Please remove my sign-off.  I didn't write or carry this patch.

If you want to acknowledge my contribution, you can use something
like Helped-by, but it's not necessary.

[...]
> +++ b/Documentation/git-read-tree.txt
> @@ -135,10 +135,10 @@ OPTIONS
>  
>  Merging
>  -------
> -If `-m` is specified, 'git read-tree' can perform 3 kinds of
> -merge, a single tree merge if only 1 tree is given, a
> -fast-forward merge with 2 trees, or a 3-way merge if 3 trees are
> -provided.
> +If `-m` is specified, at least one tree must be given on the command
> +line.

As I mentioned before, this sentence feels redundant and doesn't fix
the real problem of the `-m` reference elsewhere in this file not
pointing to this section.

[...]
> --- a/builtin/read-tree.c
> +++ b/builtin/read-tree.c
> @@ -132,7 +132,7 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
>  		OPT_BOOL(0, "empty", &read_empty,
>  			    N_("only empty the index")),
>  		OPT__VERBOSE(&opts.verbose_update, N_("be verbose")),
> -		OPT_GROUP(N_("Merging")),
> +		OPT_GROUP(N_("Merging (needs at least one tree-ish")),

This also seems a little too much of a special detail to put in the
prominent section title.  If you run "git read-tree -h", where would
you expect to find this information?

The "git read-tree -h" output turns out to not be useful for much more
than a reminder of supported options --- it doesn't give a useful
overview of the usage, since the usage string at the start is very
long.  That's unfortunate but it seems outside the scope of this
patch.  Probably the simplest thing is to drop this hunk from the
patch.


[...]
> @@ -226,9 +226,10 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
>  		setup_work_tree();
>  
>  	if (opts.merge) {
> -		if (stage < 2)
> -			die("just how do you expect me to merge %d trees?", stage-1);
>  		switch (stage - 1) {
> +		case 0:
> +			die(_("you must specify at least one tree to merge"));
> +			break;

This part looks good.

Thanks for your patient work.
Jonathan

Re: [PATCH v3 3/3] git-filter-branch:

[Junio C Hamano]

Jean-Noel Avila <jn.avila@free.fr> writes:

> Subject: Re: [PATCH v3 3/3] git-filter-branch:

Forgot the body of the single-liner summary?  In the meantime I'll
queue this as:

	Subject: git-filter-branch: be more direct in an error message

>  test -s "$tempdir"/heads ||
> -	die "Which ref do you want to rewrite?"
> +	die "You must specify a ref to rewrite."

Sounds OK, even though this (both the old and the new phrasing)
makes me wonder if the program can rewrite only one ref, or it can
accept more than one.

Thanks.

Re: [PATCH v3 2/3] read-tree -m: make error message for merging 0 trees less smart aleck

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

>> @@ -226,9 +226,10 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
>>  		setup_work_tree();
>>  
>>  	if (opts.merge) {
>> -		if (stage < 2)
>> -			die("just how do you expect me to merge %d trees?", stage-1);
>>  		switch (stage - 1) {
>> +		case 0:
>> +			die(_("you must specify at least one tree to merge"));
>> +			break;
>
> This part looks good.

Thanks.  Modulo _(""); I do not think other messages from read-tree
are marked for i18n (yet).

Re: [PATCH v3 2/3] read-tree -m: make error message for merging 0 trees less smart aleck

[Jean-Noël AVILA]

On Friday, 12 May 2017, 15:28:53 CEST Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:
> 
> >> @@ -226,9 +226,10 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
> >>  		setup_work_tree();
> >>  
> >>  	if (opts.merge) {
> >> -		if (stage < 2)
> >> -			die("just how do you expect me to merge %d trees?", stage-1);
> >>  		switch (stage - 1) {
> >> +		case 0:
> >> +			die(_("you must specify at least one tree to merge"));
> >> +			break;
> >
> > This part looks good.
> 
> Thanks.  Modulo _(""); I do not think other messages from read-tree
> are marked for i18n (yet).
> 

The documentation is already i18n, but not the dying messages. This can take place in a specific patch series.

Thanks. Will reroll. 

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

[Jean-Noel Avila]

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
that 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 <jn.avila@free.fr>
---
 builtin/am.c       | 5 +++--
 builtin/checkout.c | 5 ++---
 help.c             | 4 ++--
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index a95dd8b4e..dd60fad1e 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."));
 		die_user_resolve(state);
 	}
 
@@ -1940,7 +1940,8 @@ 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 should 'git add' each file with resolved conflicts to mark them as such.\n"
+			"You might run `git rm` on a file to accept \"deleted by them\" for it."));
 		die_user_resolve(state);
 	}
 
diff --git a/builtin/checkout.c b/builtin/checkout.c
index bfa5419f3..85c04d252 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1286,9 +1286,8 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		 * new_branch && argc > 1 will be caught later.
 		 */
 		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?"),
-			    opts.new_branch, argv[0]);
+			die(_("'%s' is not a commit and a branch '%s' cannot be created from it"),
+				argv[0], opts.new_branch);
 
 		if (opts.force_detach)
 			die(_("git checkout: --detach does not take a path argument '%s'"),
diff --git a/help.c b/help.c
index bc6cd19cf..a07f01e6f 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 similar command is",
+			      "\nThe most similar commands are",
 			   n));
 
 		for (i = 0; i < n; i++)
-- 
2.13.0

[PATCH v4 2/3] read-tree -m: make error message for merging 0 trees less smart aleck

[Jean-Noel Avila]

"git read-tree -m" requires a tree argument to name the tree to be
merged in.  Git uses a cutesy error message to say so and why:

    $ git read-tree -m
    warning: read-tree: emptying the index with no arguments is
    deprecated; use --empty
    fatal: just how do you expect me to merge 0 trees?
    $ git read-tree -m --empty
    fatal: just how do you expect me to merge 0 trees?

When lucky, that could produce an ah-hah moment for the user, but it's
more likely to irritate and distract them.

Instead, tell the user plainly that the tree argument is
required. Also document that more than 3 trees can be merged.

Signed-off-by: Jean-Noel Avila <jn.avila@free.fr>
---
 Documentation/git-read-tree.txt | 7 +++----
 builtin/read-tree.c             | 5 +++--
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-read-tree.txt b/Documentation/git-read-tree.txt
index ed9d63ef4..7e20b0c21 100644
--- a/Documentation/git-read-tree.txt
+++ b/Documentation/git-read-tree.txt
@@ -135,10 +135,9 @@ OPTIONS
 
 Merging
 -------
-If `-m` is specified, 'git read-tree' can perform 3 kinds of
-merge, a single tree merge if only 1 tree is given, a
-fast-forward merge with 2 trees, or a 3-way merge if 3 trees are
-provided.
+If `-m` is specified, 'git read-tree' can perform 3 kinds of merge, a
+single tree merge if only 1 tree is given, a fast-forward merge with 2
+trees, or a 3-way merge if 3 or more trees are provided.
 
 
 Single Tree Merge
diff --git a/builtin/read-tree.c b/builtin/read-tree.c
index 23e212ee8..383442567 100644
--- a/builtin/read-tree.c
+++ b/builtin/read-tree.c
@@ -226,9 +226,10 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
 		setup_work_tree();
 
 	if (opts.merge) {
-		if (stage < 2)
-			die("just how do you expect me to merge %d trees?", stage-1);
 		switch (stage - 1) {
+		case 0:
+			die("you must specify at least one tree to merge");
+			break;
 		case 1:
 			opts.fn = opts.prefix ? bind_merge : oneway_merge;
 			break;
-- 
2.13.0

[PATCH v4 3/3] git-filter-branch: be more direct in an error message

[Jean-Noel Avila]

git-filter-branch requires the specification of a branch by one way or
another. If no branch appears to have been specified, we know the user
got the usage wrong but we don't know what they were trying to do ---
e.g. maybe they specified the ref to rewrite but in the wrong place.

In this case, just state that the branch specification is missing.

Signed-off-by: Jean-Noel Avila <jn.avila@free.fr>
---
 git-filter-branch.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 2b8cdba15..aafaf708d 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -239,7 +239,7 @@ git rev-parse --no-flags --revs-only --symbolic-full-name \
 sed -e '/^^/d' "$tempdir"/raw-heads >"$tempdir"/heads
 
 test -s "$tempdir"/heads ||
-	die "Which ref do you want to rewrite?"
+	die "You must specify a ref to rewrite."
 
 GIT_INDEX_FILE="$(pwd)/../index"
 export GIT_INDEX_FILE
-- 
2.13.0

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

[Junio C Hamano]

Thanks, all three patches look good.  Will queue.

Let's merge them to 'next' soonish and eventually down to 'master'
and 'maint'.

Thanks.

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

[Johannes Sixt]

Am 13.05.2017 um 00:36 schrieb Junio C Hamano:
> Thanks, all three patches look good.  Will queue.
> 
> Let's merge them to 'next' soonish and eventually down to 'master'
> and 'maint'.

The patches change translated strings. You should probably wait for an 
update of their translations before you release a maintenance version 
with these changes.

-- Hannes

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

[Junio C Hamano]

Johannes Sixt <j6t@kdbg.org> writes:

> Am 13.05.2017 um 00:36 schrieb Junio C Hamano:
>> Thanks, all three patches look good.  Will queue.
>>
>> Let's merge them to 'next' soonish and eventually down to 'master'
>> and 'maint'.
>
> The patches change translated strings. You should probably wait for an
> update of their translations before you release a maintenance version
> with these changes.

Yup, thanks.