Re: [PATCH 1/5] notes-utils: handle boolean notes.rewritemode correctly

[David Kastrup <dak@gnu.org>]

Jeff King <peff@peff.net> writes:

> On Tue, Feb 18, 2014 at 09:41:51AM +0100, David Kastrup wrote:
>
>> gcc's flow analysis works with the same data as humans reading the
>> code.  If there is no information content in the function call, it makes
>> more sense to either making it void.
>
> The point of error() returning a constant -1 is to use this idiom:
>
>   if (something_failed)
>           return error("this will get printed, and we get a -1 return");

    if (something_failed)
        return error("this will get printed"), -1;

Not much of an idiom, no insider logic hidden to both compiler and
reader.

>>From a code perspective it's pointless. You could "just" write:
>
>   if (something_failed) {
>           error(...);
>           return -1;
>   }
>
> which is equivalent. But the point is that the former is shorter and a
> little more readable, assuming you are familiar with the idiom.

Assuming that.

>> One can always explicitly write
>> 
>>   (config_error_nonbool("panic-when-assailed"), -1)
>
> Yes, but again, the point is readability. Doing that at each callsite is
> ugly and annoying.

I consider insider semantics ugly and annoying.  To each his own.

> You are the first person to ask about it, so there is no stock answer.
> However, everything I told you was in the commit messages and the list
> archive already. We can also avoid questions being asked by using
> those tools.

It's further raising the barriers for contributors if they are expected
to have studied the last few years in the mailing archive.  And the
skills needed for that kind of study are mostly unrelated to good
programming skills.

So I am less than convinced that this is among the coding practices that
can be expected to attract/scare away potential contributors in
proportion to their expected capability of advancing/hindering the
project.

It's not me running the shop, so it's nothing more than an opinion.

-- 
David Kastrup