On Wed, Nov 30, 2016 at 8:35 PM, Mike Gilbert <flop...@gentoo.org> wrote: > On Wed, Nov 30, 2016 at 7:11 AM, konsolebox <konsole...@gmail.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
