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

["Kerry, Richard" <richard.kerry@atos.net>]

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.