Re: [PATCH 2/2] ident: make the useConfigOnly error messages more informative

From: Marios Titas <redneb@gmx.com>
Date: Mar 30, 2016, 23:25 UTC
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:

Show 22 quoted lines

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