Re: [edk2-devel] edk2 uncrustify update (73.0.8)?

["Pedro Falcato" <pedro.falcato@gmail.com>]

On Fri, Nov 17, 2023 at 9:08 AM Laszlo Ersek <lersek@redhat.com> wrote:
>
> On 11/16/23 09:29, Pedro Falcato wrote:
> > On Tue, Nov 14, 2023 at 3:01 PM Laszlo Ersek <lersek@redhat.com> wrote:
> >>
> >> On 11/13/23 22:33, Pedro Falcato wrote:
> >>> On Mon, Nov 13, 2023 at 8:37 PM Rebecca Cran <rebecca@bsdio.com> wrote:
> >>>>
> >>>> On 11/13/2023 1:08 PM, Michael Kubacki wrote:
> >>>>> Yes. I just did it. It is relatively minor and impacts expected code
> >>>>> areas.
> >>>>>
> >>>>> https://github.com/tianocore/edk2/pull/5043/files
> >>>>
> >>>>
> >>>> Could you update .git-blame-ignore-revs please?
> >>>
> >>> You can't do that until the merge is done, since we use
> >>> rebase-and-merge for tianocore (and rebases do not keep stable commit
> >>> hashes).
> >>> But I would plead that this should not get merged in general :/
> >>
> >
> > Laszlo!
> >
> > Sorry for the delay.
> >
> >> Seeing the cumulative diff in that PR, do you have specific
> >> counter-arguments?
> >
> > Well, my counter-argument is that formatting is becoming a topic of
> > its own. I used to be very pro-formatter but if this leads to either
> > 1) eyesore or 2) weird churn every now and then,
> > I feel like we should reconsider the current approach. I feel like all
> > formatting (manual or automated) is fine as long as it's:
> > 1) Visually consistent with the codebase's style
> > 2) Not horrendous to look at
>
> That was what we did first, for several years. It wasn't working well.
>
> An edk2 coding style document had always existed, but it had sufficient
> corner cases that formatting had *always* been a topic of its own. In
> particular, point (2) "not horrendous to look at" is impossible to define.
>
> - edk2 mainly uses a Windows-like coding style, which is immediately
> horrendous to anyone coming from a Linux background, at first

FWIW, this is a point so annoying that normal formatting tools don't
work. It makes you need a fork of an obscure formatter (I know 1
project that uses uncrustify - EDK2...), instead of something more
conventional such as clang-format.
And, FWIW, I've seen a couple of approximations of the NT kernel style
for clang-format.

>
> - edk2 uses extremely long lines in some modules (200+ characters). I
> personally use 80 character lines (for good reason -- not the greatest
> eyesight, so I need to use a relatively large font, and I like to place
> two columns of source code on my screen side-by-side, for diffing or
> comparing otherwise). I've received multiple "buy a larger monitor" or
> "use two monitors at the same time", and those don't work for me either.
> Some people are super productive with 30" monitors, I'm not. Therefore,
> "long lines or not" can never be decided by democracy, only by decree.
> uncrustify is decree.
>
> - somewhat as a consequence of my avoidance of long lines, I tended to
> break up long function calls earlier / more frequently than other
> developers. In order to conserve *vertical* screen real estate, I used a
> "condensed" multi-line argument style for function calls, in OvmfPkg and
> ArmVirtPkg. (The official multi-line style is more wasteful.) That
> wasn't working for other project participants. Uncrustify now does not
> permit my preferred (condensed) style, but at least there are no more
> debates about it.
>
> Your premise is that those two points form a stable foundation, but they
> don't. We tried.

I may be too naive, but with a *well defined* coding style and a
formatter to *help*, I don't see how things would end up poorly.
I think a formatter's job should be to *aid*, to not enforce. These
things are fallible. For instance, clang-format eyesore out of one of
my personal projects:

#define ONES ((size_t) -1 / UCHAR_MAX)
#define HIGHS (ONES * (UCHAR_MAX / 2 + 1))
#define HASZERO(x) (((x) -ONES) & ~(x) &HIGHS)

(may be hard to see in email, but note how clang-format has no idea
how any macro works, so (x) - ONES becomes (x) -ONES and <expr> &
HIGHS becomes <expr> &HIGHS...)

For instance, I think Linux manages this quite nicely. They provide a
.clang-format you can use, and the formatting generally ends up fine.
But you're not expected to re-format every couple of changes, not
everyone is at the same formatter version (if at all!) and "human
thinks looks fine" has higher priority over "*formatter* thinks it
looks fine".

-- 
Pedro


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111639): https://edk2.groups.io/g/devel/message/111639
Mute This Topic: https://groups.io/mt/102559740/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-