Re: [PATCH] Fixing unclear messages

From: Alexander Shopov <ash@kambanaria.org>
Date: Aug 11, 2014, 09:13 UTC
Message-ID: <CAP6f5MmE4ae81DZu-gbaax4nSh4WMcWCk453wJBfC4mKvmsfxA@mail.gmail.com>
In-Reply-To: <xmqqsil37v4i.fsf@gitster.dls.corp.google.com>

>> -                     printf_ln(_("Huh (%s)?"), (*ptr)->buf);
>> +                     printf_ln(_("Wrong choice (%s). Choose again."), (*ptr)->buf);
> Why is this an improvement?

Because 'Huh?' without intonation, gesture or a frown provided by a human being is hard to understand. It does not state that it is the choice the user provided is wrong and does not prompt the user for the next action.

>> -             only_include_assumed = _("Clever... amending the last one with dirty index.");
>> +             only_include_assumed = _("You are amending the last commit only. There are additional changes in the index.");
> Why is this an improvement?

...

> Besides, "amending the last commit only."  implies ...
> ... does not convey any extra information ...
> ...It may be time to remove these messages, by the way. ...

You say that my change does not tangibly improve on an initially unclear and already obsolete message, am I right? I prefer the messages to be removed then.

>> +             die(_("fatal error in function \"parse_pack_objects\". This is a bug in Git. Please report it to the developers with an e-mail to git@vger.kernel.org."));
> It probably should be spelled die("BUG: ...").
>> +                     die(_("fatal error in function \"conclude_pack\". This is a bug in Git. Please report it to the developers with an e-mail to git@vger.kernel.org."));
> Likewise.

I have no stand on the issue "fatal error" vs "BUG", if you prefer "BUG" I can reword. There was a suggestion to have a separate function that is meant to echo messages when there are genuine bugs in Git (impossible cases happening, invariants breaking, etc.) This will allow factoring out the clause "This is a bug in Git. Please report it to the developers with an e-mail to git@vger.kernel.org." as a single message and I prefer that for ease of maintenance.

>> +             die(_("wrong format for the \"in-reply-to\" header: %s"), msg_id);
> Why is s/insane/wrong format/ an improvement?

Because it is more factual and narrows down what is wrong. "Insane" could mean many different things.

>> -             die(_("Two output directories?"));
>> +             die(_("Maximum one output directory is allowed."));
> Why is it an improvement?

Because the question only implies that the problem is the number of directories but says nothing how many directories there should be (0, 1, 3... 100?)

>> -     printf(_("Wonderful.\n"));
>> +     printf(_("The first part of the trivial merge finished successfully
> Huh?

I am not sure what you mean or your objection would be, perhaps I am misreading the source of Git. The message appears as a part of sequence during merge when the first stage passes successfully but there still could be a case that makes the whole merge impossible. What does "Wonderful" mean in a sequence of steps git is doing for you.

You say "I would buy s/something/BUG: &/;, but I do not think we want to apply most of the others." Does this mean the following changes are totally unwelcome or you (plural, as corresponds to "we) want me to resubmit them and substantiate changes on a message by message base?

-Nope.\n +Merge was not successful.\n

- ???
+ unknown state

-no tag message? +missing tag message

-?? what are you talking about? +unknown command. Only "start", "good", "bad" and "skip" are possible.

-Unprocessed path??? %s +The path "%s" was not processed but it had to be. This is a bug in Git. Please report it to the developers with an e-mail to git@vger.kernel.org.

Kind regards: al_shopov