Re: [PATCHv3] refs.c: enable large transactions

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Apr 24, 2015, 18:12 UTC
Message-ID: <20150424181236.GK5467@google.com>
In-Reply-To: <CAGZ79kYGDOUgzKmQOLAXkYYb-HZJCw3Y=iSLXWHVXcQ0pAiDBw@mail.gmail.com>

Stefan Beller wrote:

> I understood that you were talking about Could being capitalized.
> Though it's distributed 30/70 which hints to me we do not care in
> enough to explain the capitalized letters as slip-throughs on review
> or such, but it looks like the authors choice to me.

Lowercase, brief, with no period at the end is the convention. This is user-facing UI, so consistency matters. It's not author's choice.

Existing examples of capitalized messages fall into a few categories:

 - shell scripts used to tend to use the capitalized form, and when
   they got rewritten as builtins the messages weren't cleaned up at
   the same time

 - some patch authors have different habits and it wasn't worth going
   back and forth to fix it (but the convention still stands, and the
   result of a program that can't decide how to talk to the user is
   still harmful)

 - once there are a few examples, they get copy/pasted and imitated

> I think it's a mistake to s/Could/could/g for all errors in the code base
> as it reduces the information provided in the error messages.

This seems like an after-the-fact justification for a mistake.

Often there is a choice about whether the caller or the callee to a function prints an error. If the caller does, it can say more about the context. If the callee does, the message is in one place and can be tweaked more easily to be more useful.

To get the benefits of both, we could print a backtrace with every error. That way, the callee can describe the error in one place but the context is all there. But I'm really glad we don't do that!

The main purpose of most error messages is instructional: they give information to the user in a way that is abstracted from the implementation (the user doesn't care what function it happened in!) that tells them what happened and what they can do next. Gratuitous inconsistency in formatting doesn't help with that.

Actually, I wouldn't mind an environment variable that tells error() to include a backtrace if someone wants it. (See backtrace(3) for implementation hints if interested in doing that.)

> Just 3 days ago ("Regular Rebase Failure"). I used different
> capitalization to get a better understanding where the error may be.

Wouldn't it be better if there weren't so many similar messages produced in different contexts in the first place? (I.e., this suggests to me that we should move more in the direction of callee-produces-error.)

Sorry, that was a long way to say very little. I just wanted to emphasize that when a UI inconsistency has a useful side effect, while that can be useful to understand and learn from, we should not be afraid to fix it. And to clear up any ambiguity about git's error message convention. :)

Thanks and hope that helps, Jonathan