Re: [PATCH] commit: reword --author error message

2015-01-16 Thread Junio C Hamano
Eric Sunshine  writes:

> On Fri, Jan 16, 2015 at 1:35 PM, Junio C Hamano  wrote:
>> "Philip Oakley"  writes:
>>
>>> die(_("--author '%s': not 'Name ', nor matches any existing
>>> author"));
>>
>> Sounds good.  Thanks.
>
> To further bikeshed (particularly if "nor" is in the mix):
>
> neither 'Name ' nor a match for an existing author

Short and sweet ;-)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] commit: reword --author error message

2015-01-16 Thread Eric Sunshine
On Fri, Jan 16, 2015 at 1:35 PM, Junio C Hamano  wrote:
> "Philip Oakley"  writes:
>
>> die(_("--author '%s': not 'Name ', nor matches any existing
>> author"));
>
> Sounds good.  Thanks.

To further bikeshed (particularly if "nor" is in the mix):

neither 'Name ' nor a match for an existing author
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] commit: reword --author error message

2015-01-16 Thread Junio C Hamano
"Philip Oakley"  writes:

> die(_("--author '%s': not 'Name ', nor matches any existing
> author"));

Sounds good.  Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] commit: reword --author error message

2015-01-16 Thread Philip Oakley

From: "Junio C Hamano" 

Jeff King  writes:


On Thu, Jan 15, 2015 at 09:48:26AM -0800, Junio C Hamano wrote:

> Rename the error message to make it clearer that the failure has 
> two

> reasons in this case:
> "Bad --author parameter '%s': neither completely wellformed nor 
> part of

> an existing one"

You are trying to help a user who thought "Who www.where.com" was a
valid thing to pass to --author; "it is not completely wellformed"
is not very helpful without making her realize what in that bogus
string is not "completely well-formed".

Perhaps

  "--author '%s' is not 'Name ' and no existing author 
matches that string"


or somesuch?


Yeah, I think that is OK. It is kind of clunky to read, but it 
contains

all of the necessary information to lead the user in the right
direction.


Indeed it is clunky and not in the usual format.

This might be better, perhaps?

   die(_("--author '%s': not 'Name ' and matches no existing 
author"));

/Bikeshed
die(_("--author '%s': not 'Name ', nor matches any existing 
author"));
The 'not/and' construct didn't feel right here in the UK, but then many 
don't understand 'nor' either.

--
Philip



--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] commit: reword --author error message

2015-01-16 Thread Junio C Hamano
Jeff King  writes:

> On Thu, Jan 15, 2015 at 09:48:26AM -0800, Junio C Hamano wrote:
>
>> > Rename the error message to make it clearer that the failure has two
>> > reasons in this case:
>> > "Bad --author parameter '%s': neither completely wellformed nor part of
>> > an existing one"
>> 
>> You are trying to help a user who thought "Who www.where.com" was a
>> valid thing to pass to --author; "it is not completely wellformed"
>> is not very helpful without making her realize what in that bogus
>> string is not "completely well-formed".
>> 
>> Perhaps
>> 
>>   "--author '%s' is not 'Name ' and no existing author matches that 
>> string"
>> 
>> or somesuch?
>
> Yeah, I think that is OK. It is kind of clunky to read, but it contains
> all of the necessary information to lead the user in the right
> direction.

Indeed it is clunky and not in the usual format.

This might be better, perhaps?

die(_("--author '%s': not 'Name ' and matches no existing author"));

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] commit: reword --author error message

2015-01-16 Thread Jeff King
On Thu, Jan 15, 2015 at 09:48:26AM -0800, Junio C Hamano wrote:

> > Rename the error message to make it clearer that the failure has two
> > reasons in this case:
> > "Bad --author parameter '%s': neither completely wellformed nor part of
> > an existing one"
> 
> You are trying to help a user who thought "Who www.where.com" was a
> valid thing to pass to --author; "it is not completely wellformed"
> is not very helpful without making her realize what in that bogus
> string is not "completely well-formed".
> 
> Perhaps
> 
>   "--author '%s' is not 'Name ' and no existing author matches that 
> string"
> 
> or somesuch?

Yeah, I think that is OK. It is kind of clunky to read, but it contains
all of the necessary information to lead the user in the right
direction.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] commit: reword --author error message

2015-01-15 Thread Junio C Hamano
Michael J Gruber  writes:

> If an --author argument is specified but does not contain a '>' then git tries
> to find the argument within the exiting authors; and gives the error
> message "No existing author found with '%s'" if there is no match.
>
> This is confusing for users who try to specify a valid complete author
> name.

I suspect that you meant s/a valid/an invalid/, as if it is valid,
it cannot not contain '>' (after all, '>' is merely a rough
approximation to check if it is "Name " format).

> Rename the error message to make it clearer that the failure has two
> reasons in this case:
> "Bad --author parameter '%s': neither completely wellformed nor part of
> an existing one"

You are trying to help a user who thought "Who www.where.com" was a
valid thing to pass to --author; "it is not completely wellformed"
is not very helpful without making her realize what in that bogus
string is not "completely well-formed".

Perhaps

  "--author '%s' is not 'Name ' and no existing author matches that 
string"

or somesuch?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] commit: reword --author error message

2015-01-15 Thread Michael J Gruber
Jeff King schrieb am 15.01.2015 um 15:31:
> On Thu, Jan 15, 2015 at 03:23:08PM +0100, Michael J Gruber wrote:
> 
>> If an --author argument is specified but does not contain a '>' then git 
>> tries
>> to find the argument within the exiting authors; and gives the error
>> message "No existing author found with '%s'" if there is no match.
>>
>> This is confusing for users who try to specify a valid complete author
>> name.
>>
>> Rename the error message to make it clearer that the failure has two
>> reasons in this case:
>> "Bad --author parameter '%s': neither completely wellformed nor part of
>> an existing one"
> 
> I really like the intent of this patch, but I actually find the new
> message even more confusing.

The main observation is that the current error message is given only
when both interpretations (complete ident, match ident) fail, and the
error message conveys only one when it should do both. I don't care
about the wording either.

> Is this a time when we could use hint() to give a multi-line explanation
> (and probably a matching advice.* config)? Like:
> 
>   hint: If the --author parameter contains angle brackets ("<>"), it
>   hint: is treated as a literal name/email pair to use. If not, then
>   hint: the history is searched for an existing matching author.
> 
> or something?
> 
> -Peff
> 

Well, this basically copies the man page paragraph for that option. I
don't want to set a(nother) precedent for doing this and create yet
another config knob.

The alternative would be to just say "Bad --author parameter '%s'" (or
"Invalid..."), as we do in most cases, and force the user to check the
man page for the definition of "valid". I'm beginning to prefer this
minimalistic approach...

Michael
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] commit: reword --author error message

2015-01-15 Thread Jeff King
On Thu, Jan 15, 2015 at 03:23:08PM +0100, Michael J Gruber wrote:

> If an --author argument is specified but does not contain a '>' then git tries
> to find the argument within the exiting authors; and gives the error
> message "No existing author found with '%s'" if there is no match.
> 
> This is confusing for users who try to specify a valid complete author
> name.
> 
> Rename the error message to make it clearer that the failure has two
> reasons in this case:
> "Bad --author parameter '%s': neither completely wellformed nor part of
> an existing one"

I really like the intent of this patch, but I actually find the new
message even more confusing.

Is this a time when we could use hint() to give a multi-line explanation
(and probably a matching advice.* config)? Like:

  hint: If the --author parameter contains angle brackets ("<>"), it
  hint: is treated as a literal name/email pair to use. If not, then
  hint: the history is searched for an existing matching author.

or something?

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] commit: reword --author error message

2015-01-15 Thread Michael J Gruber
Michael J Gruber schrieb am 15.01.2015 um 15:23:
> If an --author argument is specified but does not contain a '>' then git tries
> to find the argument within the exiting authors; and gives the error
> message "No existing author found with '%s'" if there is no match.

Oh well, I'm bracing already for the comments on that entertaining
typo... Can I buy an 's', please?

> This is confusing for users who try to specify a valid complete author
> name.
> 
> Rename the error message to make it clearer that the failure has two
> reasons in this case:
> "Bad --author parameter '%s': neither completely wellformed nor part of
> an existing one"
> 
> (This codepath is touched only when we know already that the argument
> cannot be a completely wellformed author ident.)
> 
> Signed-off-by: Michael J Gruber 
> ---
>  builtin/commit.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 7d90c35..851e2c5 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -1056,7 +1056,7 @@ static const char *find_author_by_nickname(const char 
> *name)
>   clear_mailmap(&mailmap);
>   return strbuf_detach(&buf, NULL);
>   }
> - die(_("No existing author found with '%s'"), name);
> + die(_("Bad --author parameter '%s': neither completely wellformed nor 
> part of an existing one"), name);
>  }
>  
>  
> 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html