Re: [meta-oe][PATCH] recipes: Unify indentation

[Joe MacDonald <Joe.MacDonald@windriver.com>]

[-- Attachment #1: Type: text/plain, Size: 4712 bytes --]

[Re: [oe] [meta-oe][PATCH] recipes: Unify indentation] On 13.04.15 (Mon 15:07) Martin Jansa wrote:

> On Mon, Apr 15, 2013 at 08:42:59AM -0400, Joe MacDonald wrote:
> > [Re: [oe] [meta-oe][PATCH] recipes: Unify indentation] On 13.04.14 (Sun 18:30) Koen Kooi wrote:
> > 
> > > -----BEGIN PGP SIGNED MESSAGE-----
> > > Hash: SHA1
> > > 
> > > Op 14-04-13 00:12, Martin Jansa schreef:
> > > > * This change is only aesthetic (unlike indentation in Python tasks). *
> > > > Some recipes were using tabs. * Some were using 8 spaces. * Some were
> > > > using mix or different number of spaces. * Make them consistently use 4
> > > > spaces everywhere. * Yocto styleguide advises to use tabs (but the only
> > > > reason to keep tabs is the need to update a lot of recipes). Lately this
> > > > advice was also merged into the styleguide on the OE wiki. * Using 4
> > > > spaces in both types of tasks is better because it's less error prone
> > > > when someone is not sure if e.g. do_generate_toolchain_file() is Python
> > > > or shell task and also allows to highlight every tab used in .bb, .inc,
> > > > .bbappend, .bbclass as potentially bad (shouldn't be used for indenting
> > > > of multiline variable assignments and cannot be used for Python tasks).
> > > > 
> > > > Signed-off-by: Martin Jansa <Martin.Jansa@gmail.com>
> > > 
> > > I still hate spaces for shell methods, but I support the reasons behind it, so:
> > > 
> > > Acked-by: Koen Kooi <koen@dominion.thruhere.net>
> > 
> > I completely agree.  The only spot where I see this as being not optimal
> > is something like this (hunk simplified for clarity):
> > 
> >  PACKAGES += "${PN}-ndisc6 ${PN}-tcpspray6 ${PN}-rdisc6 \
> > -            ${PN}-tcptraceroute6 ${PN}-rltraceroute6 \
> > -             ${PN}-tracert6 ${PN}-rdnssd ${PN}-misc"
> > +    ${PN}-tcptraceroute6 ${PN}-rltraceroute6 \
> > +    ${PN}-tracert6 ${PN}-rdnssd ${PN}-misc"
> 
> the off-by-one was probably reason why it was moved to 4-space
> indentation. Sometimes it wasn't off-by-one, but when tabs and spaces
> were mixed to indent them (with ${PN} aligned) then after replacing tabs
> with 4 spaces it was probably broken a bit more.

Ah, okay.

> 
> > The former state wasn't great, but in general if I'm doing this type of
> > thing, I'll tend to align them thus:
> > 
> > PACKAGES += "${PN}-ndisc6 ${PN}-tcpspray6 ${PN}-rdisc6 \
> >              ${PN}-tcptraceroute6 ${PN}-rltraceroute6 \
> >              ${PN}-tracert6 ${PN}-rdnssd ${PN}-misc"
> 
> It it was aligned like this (after replacing tabs) I was keeping it.
> 
> Examples like:
> PACKAGES += "${PN}-ndisc6 ${PN}-tcpspray6 ${PN}-rdisc6 \
>           ${PN}-tcptraceroute6 ${PN}-rltraceroute6 \
>           ${PN}-tracert6 ${PN}-rdnssd ${PN}-misc"
> were unified to use only 4 spaces.
> 
> The same for less spaces, e.g.
> PACKAGES += "${PN}-ndisc6 ${PN}-tcpspray6 ${PN}-rdisc6 \
>   ${PN}-tcptraceroute6 ${PN}-rltraceroute6 \
>   ${PN}-tracert6 ${PN}-rdnssd ${PN}-misc"
> 
> I was also tempted to remove trailing spaces and move closing quote to
> be always on new line (easier to add/remove lines when all end with \).
> But that was a bit more difficult for my shell/sed monkeys, because
> shell tasks has many matching '"$' lines following line which also
> starts with spaces. So I kept closing quotes as they were, only on lines
> with only quote I've moved it to beginning.
> 
> PACKAGES += "${PN}-ndisc6 ${PN}-tcpspray6 ${PN}-rdisc6 \
>     ${PN}-tcptraceroute6 ${PN}-rltraceroute6 \
>     ${PN}-tracert6 ${PN}-rdnssd ${PN}-misc \
> "
> looks better than
> PACKAGES += "${PN}-ndisc6 ${PN}-tcpspray6 ${PN}-rdisc6 \
>     ${PN}-tcptraceroute6 ${PN}-rltraceroute6 \
>     ${PN}-tracert6 ${PN}-rdnssd ${PN}-misc \
>        "

Yep, I agree.

Okay, so thanks for the detailed explanation, Martin.  I'm quite happy
with this.

-J.

> 
> Some changes were manual (git grep to find suspicious lines and then
> check them manually) so it isn't 100% perfect but as you said, it's
> better then it was before.
> 
> git diff -w shows only few removed empty lines (e.g. begining or end of
> shell task) and 2-3 lines which I've manually splitted to multiline.
> 
> With 3 acks already I'll merge this together with systemd changes today.
> 
> We should update OE styleguide after this :).
> 
> > Probably leaving such things as they are in the tree is more trouble
> > than it's worth, but we could, I'd like to avoid restyling after a line
> > continuation.  I won't object to the proposal as it stands, though,
> > since on the whole it looks to be doing much more good than harm.
> 
-- 
-Joe MacDonald.
:wq

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 205 bytes --]