Re: [gentoo-dev] new eclass: tmpfiles.eclass round 4

[konsolebox]

On Wed, Nov 30, 2016 at 8:35 PM, Mike Gilbert <floppym@g.o> wrote:
> On Wed, Nov 30, 2016 at 7:11 AM, konsolebox <konsolebox@×××××.com> wrote:
>>>> I also prefer some things this way:
>>>>
>>>> - Indent the contents of the first `if` block for consistency's sake,
>>>> and less confusion.
>>>
>>> I disagree; indenting the entire eclass is silly and does not really
>>> improve readability. Also, this is a very common pattern found in
>>> other eclasses.
>>
>> I prefer following consistency before anything else.  And it's just
>> uncommon and odd, but it's not silly.  Imagine if you use another `if`
>> block on the second level where more functions are defined.  Would you
>> also not indent that?
>
> That first "if" is a bit of a special case; it's only there to prevent
> the code from being executed more than once in global scope. This
> provides a minor speed boost when the eclass gets sourced more than
> once, and makes sure that any global variables are only set once.

OT: inherit() should have already been able to have a non-hacky
solution of making sure that eclass files are loaded only once.  Bash
4.2 has already supported global declaration of variables with -g (in
case the flag variable would need to be declared inside a function),
and associative arrays with -A.  Even older versions of Bash (<4.0)
can make use of multiple variables with a common prefix to imitate
associative arrays if compatibility is needed.  It could just check
BASH_VERSINFO if it has to downgrade to a less-efficient method.

Besides no longer having to rely on a special `if` block, it probably
would also make loading of ebuilds faster since script codes that are
referenced multiple times wouldn't have to be re-parsed again and
again by bash.  It's scary to imagine how it happens in all ebuilds
that's read in portage, but maybe repeated loading of eclass files
doesn't always happen, who knows.  (I think it's still significant
even if the dependencies are already cached.)

> I think of it as being similar to pre-processor statement you would
> find in a C header file; one does not generally indent all the code
> within a C header file, because it just introduces a big chunk of
> whitespace that does not improve code readability.

I did as well, but I'll still prefer to have it indented, because it
avoids trying to know what the last lone `fi` is for, if you haven't
seen the first `if` condition yet.  If they are indented, your mind is
automatically set to know that they are within a conditional block,
and no surprise happens.  Maybe that's not the case if people are
already used to seeing those setup, like in eclass files, but I'm not,
so it does help improve readability in my case.

To be honest I find it painful to see anything inside a block to be
not indented, no matter what the reason is.  You can say that they
look like preprocessors in C, and preprocessors don't make codes
indent, but preprocessors are not (functional) codes, and they are
naturally recognized to have their own "indentation space" or
patterns, unlike the shell's `if`.

But anyway, I think I find it understandable now, in the context of
eclass files.

-- 
konsolebox