From: Marios Titas <redneb@gmx.com>
Date: Wed, 30 Mar 2016 23:25:52 GMT
Subject: Re: [PATCH 2/2] ident: make the useConfigOnly error messages more informative
Message-ID: <20160330232552.GA31861@zeno>
In-Reply-To: <xmqqlh4zr492.fsf@gitster.mtv.corp.google.com>

On Wed, Mar 30, 2016 at 03:27:05PM -0700, Junio C Hamano wrote:
>Marios Titas <redneb@gmx.com> writes:
>
>> -			    && !(ident_config_given & IDENT_NAME_GIVEN))
>> -				die("user.useConfigOnly set but no name given");
>> +			    && !(ident_config_given & IDENT_NAME_GIVEN)) {
>> +				fputs(env_hint, stderr);
>> +				die("no name was given and auto-detection is disabled
>
>Hmph.  I do not think that this is making the message "more
>informative".
>
>When a user hits this error, the old message allowed the user to
>easily see how to toggle the "disable auto-detection" bit off to let
>the code continue by telling the name of the configuration, but the
>updated message hides that name, making it harder for the user to
>disable the disabling of auto-detection.
>
>I can buy the argument that this change helps the user by making the
>message "less" informative, though.  By discouraging the users from
>toggling the user.useConfigOnly bit off, it indirectly makes the
>other option to work around this error condition, i.e. giving a name
>more explicitly, more appetizing.

Yeah, maybe informative is not the right word. What I meant is that it 
directs the user to do the "git config user.name" thing, which is likely 
the most appropriate course of action in this situation. In any event, I 
think printing the env_hint message would be really helpful in this 
case.