[edk2-devel] Maybe it is time to update code style?

[edk2-devel] Maybe it is time to update code style?

[Marcin Juszkiewicz]

I feel sick each time I have to edit EDK2 code.

All those INF, DEC, DSC, FDF, XYZ files are something I do not even try
to understand, just got minimal knowledge what goes where by asking Leif
(thanks a lot!) and observing build error messages.

I got used to UINTN and other weird variable types. No idea where from
they came from (MS Windows?)


But what sucks more is those 2 spaces used for indenting code. Took me
a while to reming myself when I used such. It was in 90s, when I was
Amiga programmer and all I had was 720x480px screen.

Last time I looked at calendar it was 2023 year. Maybe it is time to move
to 4 spaces?

And can someone take a look at config of code obfuscator used for linting?
It spits out amount of information showing that noone looked at updating it
to current version:

$  uncrustify -c ../edk2/.pytool/Plugin/UncrustifyCheck/uncrustify.cfg  Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
Option<UNUM>: at ../edk2/.pytool/Plugin/UncrustifyCheck/uncrustify.cfg:48: Expected unsigned number , for 'mod_full_brace_if_chain'; got 'false'
../edk2/.pytool/Plugin/UncrustifyCheck/uncrustify.cfg:79: unknown option 'indent_func_call_edk2_style'
Option<NUM>: at ../edk2/.pytool/Plugin/UncrustifyCheck/uncrustify.cfg:212: Expected number , for 'indent_bool_paren'; got 'true'
Option<NUM>: at ../edk2/.pytool/Plugin/UncrustifyCheck/uncrustify.cfg:224: Expected number , for 'indent_comma_paren'; got 'true'
../edk2/.pytool/Plugin/UncrustifyCheck/uncrustify.cfg:241: option 'indent_sing_line_comments' is deprecated; did you want to use 'indent_single_line_comments_before' instead?
../edk2/.pytool/Plugin/UncrustifyCheck/uncrustify.cfg:270: unknown option 'align_edk2_style'
../edk2/.pytool/Plugin/UncrustifyCheck/uncrustify.cfg:426: option 'nl_func_var_def_blk' is deprecated; it has been replaced by 'nl_var_def_blk_end_func_top'.
You can also use 'nl_var_def_blk_end' for additional functionality
../edk2/.pytool/Plugin/UncrustifyCheck/uncrustify.cfg:447: option 'pp_space' is deprecated; it has been replaced by 'pp_space_after'.
Output suffix: .uncrustify
do_source_file(1524): Parsing: Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c as language C

$  uncrustify --version
Uncrustify_d-0.76.0_f


And yet it is a tool which contributors like me are supposed to use before
sending any change upstream.


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

Re: [edk2-devel] Maybe it is time to update code style?

[Michael D Kinney]

I think you may be using the wrong version of the uncrustify tool.

Is that log from a local run or EDK II CI?

https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Code-Formatting

https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Code-Formatting#installing-uncrustify

Mike

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Marcin
> Juszkiewicz
> Sent: Monday, September 25, 2023 2:03 AM
> To: devel <devel@edk2.groups.io>
> Subject: [edk2-devel] Maybe it is time to update code style?
> 
> I feel sick each time I have to edit EDK2 code.
> 
> All those INF, DEC, DSC, FDF, XYZ files are something I do not even
> try
> to understand, just got minimal knowledge what goes where by asking
> Leif
> (thanks a lot!) and observing build error messages.
> 
> I got used to UINTN and other weird variable types. No idea where from
> they came from (MS Windows?)
> 
> 
> But what sucks more is those 2 spaces used for indenting code. Took me
> a while to reming myself when I used such. It was in 90s, when I was
> Amiga programmer and all I had was 720x480px screen.
> 
> Last time I looked at calendar it was 2023 year. Maybe it is time to
> move
> to 4 spaces?
> 
> And can someone take a look at config of code obfuscator used for
> linting?
> It spits out amount of information showing that noone looked at
> updating it
> to current version:
> 
> $  uncrustify -c ../edk2/.pytool/Plugin/UncrustifyCheck/uncrustify.cfg
> Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> Option<UNUM>: at
> ../edk2/.pytool/Plugin/UncrustifyCheck/uncrustify.cfg:48: Expected
> unsigned number , for 'mod_full_brace_if_chain'; got 'false'
> ../edk2/.pytool/Plugin/UncrustifyCheck/uncrustify.cfg:79: unknown
> option 'indent_func_call_edk2_style'
> Option<NUM>: at
> ../edk2/.pytool/Plugin/UncrustifyCheck/uncrustify.cfg:212: Expected
> number , for 'indent_bool_paren'; got 'true'
> Option<NUM>: at
> ../edk2/.pytool/Plugin/UncrustifyCheck/uncrustify.cfg:224: Expected
> number , for 'indent_comma_paren'; got 'true'
> ../edk2/.pytool/Plugin/UncrustifyCheck/uncrustify.cfg:241: option
> 'indent_sing_line_comments' is deprecated; did you want to use
> 'indent_single_line_comments_before' instead?
> ../edk2/.pytool/Plugin/UncrustifyCheck/uncrustify.cfg:270: unknown
> option 'align_edk2_style'
> ../edk2/.pytool/Plugin/UncrustifyCheck/uncrustify.cfg:426: option
> 'nl_func_var_def_blk' is deprecated; it has been replaced by
> 'nl_var_def_blk_end_func_top'.
> You can also use 'nl_var_def_blk_end' for additional functionality
> ../edk2/.pytool/Plugin/UncrustifyCheck/uncrustify.cfg:447: option
> 'pp_space' is deprecated; it has been replaced by 'pp_space_after'.
> Output suffix: .uncrustify
> do_source_file(1524): Parsing:
> Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c as
> language C
> 
> $  uncrustify --version
> Uncrustify_d-0.76.0_f
> 
> 
> And yet it is a tool which contributors like me are supposed to use
> before
> sending any change upstream.
> 
> 
> 
> 



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

Re: [edk2-devel] Maybe it is time to update code style?

[Andrew Fish via groups.io]

It took me 2-3 weeks to flip my brain over to the 2 space indent. It was not so bad when I was just working on EFI stuff but if switch around between projects my editor having the wrong settings gets a lit old sometimes. 

The history of this was some of the old Intel legacy BIOS devs really pushed for the 2 spaces vs. 4 spaces so we let them have a win in the requirements. Turns out they have an old dot matrix printer with that old computer paper that had alternating colors on every line to make it easier to read the code. They pushed for 2 spaces so less code would line wrap on their team printer…..

Thanks,

Andrew Fish

> On Sep 25, 2023, at 2:03 AM, Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org> wrote:
> 
> I feel sick each time I have to edit EDK2 code.
> 
> All those INF, DEC, DSC, FDF, XYZ files are something I do not even try
> to understand, just got minimal knowledge what goes where by asking Leif
> (thanks a lot!) and observing build error messages.
> 
> I got used to UINTN and other weird variable types. No idea where from
> they came from (MS Windows?)
> 
> 
> But what sucks more is those 2 spaces used for indenting code. Took me
> a while to reming myself when I used such. It was in 90s, when I was
> Amiga programmer and all I had was 720x480px screen.
> 
> Last time I looked at calendar it was 2023 year. Maybe it is time to move
> to 4 spaces?
> 
> And can someone take a look at config of code obfuscator used for linting?
> It spits out amount of information showing that noone looked at updating it
> to current version:
> 
> $  uncrustify -c ../edk2/.pytool/Plugin/UncrustifyCheck/uncrustify.cfg  Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> Option<UNUM>: at ../edk2/.pytool/Plugin/UncrustifyCheck/uncrustify.cfg:48: Expected unsigned number , for 'mod_full_brace_if_chain'; got 'false'
> ../edk2/.pytool/Plugin/UncrustifyCheck/uncrustify.cfg:79: unknown option 'indent_func_call_edk2_style'
> Option<NUM>: at ../edk2/.pytool/Plugin/UncrustifyCheck/uncrustify.cfg:212: Expected number , for 'indent_bool_paren'; got 'true'
> Option<NUM>: at ../edk2/.pytool/Plugin/UncrustifyCheck/uncrustify.cfg:224: Expected number , for 'indent_comma_paren'; got 'true'
> ../edk2/.pytool/Plugin/UncrustifyCheck/uncrustify.cfg:241: option 'indent_sing_line_comments' is deprecated; did you want to use 'indent_single_line_comments_before' instead?
> ../edk2/.pytool/Plugin/UncrustifyCheck/uncrustify.cfg:270: unknown option 'align_edk2_style'
> ../edk2/.pytool/Plugin/UncrustifyCheck/uncrustify.cfg:426: option 'nl_func_var_def_blk' is deprecated; it has been replaced by 'nl_var_def_blk_end_func_top'.
> You can also use 'nl_var_def_blk_end' for additional functionality
> ../edk2/.pytool/Plugin/UncrustifyCheck/uncrustify.cfg:447: option 'pp_space' is deprecated; it has been replaced by 'pp_space_after'.
> Output suffix: .uncrustify
> do_source_file(1524): Parsing: Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c as language C
> 
> $  uncrustify --version
> Uncrustify_d-0.76.0_f
> 
> 
> And yet it is a tool which contributors like me are supposed to use before
> sending any change upstream.
> 
> 
> 
> 
> 



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

Re: [edk2-devel] Maybe it is time to update code style?

[Rebecca Cran via groups.io]

On 9/25/2023 3:03 AM, Marcin Juszkiewicz via groups.io wrote:
> I feel sick each time I have to edit EDK2 code.
> 
> All those INF, DEC, DSC, FDF, XYZ files are something I do not even try
> to understand, just got minimal knowledge what goes where by asking Leif
> (thanks a lot!) and observing build error messages.
> 
> I got used to UINTN and other weird variable types. No idea where from
> they came from (MS Windows?)

Given that people at Intel/Microsoft are still adding code with the 
Windows NT style function documentation, I suspect there will be quite a 
resistance to changing things.

For example, the following was added in 2019:


EFI_STATUS
GetSelfTest (
...
   )
/*++

Routine Description:

   Execute the Get Self Test ...

Arguments:

   IpmiInstance    - Data structure...

Returns:

   EFI_SUCCESS       - BMC Self test results...

--*/
{
   EFI_STATUS  Status;

Vincent provided the following explanation on Twitter:

"It's from MS, namley the coding style used in Windows NT 
https://computernewb.com/~lily/files/Documents/NTDesignWorkbook/coding.pdf 
. Ken Reneris wrote the original EFI sample at Intel after working 
having worked on various parts of NT, including the HAL, at MS. He 
brought that coding style with him to Intel."

-- 
Rebecca Cran


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

Re: [edk2-devel] Maybe it is time to update code style?

[Pedro Falcato]

On Mon, Sep 25, 2023 at 10:03 AM Marcin Juszkiewicz
<marcin.juszkiewicz@linaro.org> wrote:
>
> I feel sick each time I have to edit EDK2 code.
>
> All those INF, DEC, DSC, FDF, XYZ files are something I do not even try
> to understand, just got minimal knowledge what goes where by asking Leif
> (thanks a lot!) and observing build error messages.

I feel you. We've discussed this before (in the ML and on the UEFI
talkbox discord) and there's a large unwillingness to change the build
system, because it's "super extensible" and all that.
I don't like it, but it is what it is.

>
> I got used to UINTN and other weird variable types. No idea where from
> they came from (MS Windows?)

Yes.

>
> But what sucks more is those 2 spaces used for indenting code. Took me
> a while to reming myself when I used such. It was in 90s, when I was
> Amiga programmer and all I had was 720x480px screen.
>
> Last time I looked at calendar it was 2023 year. Maybe it is time to move
> to 4 spaces?
>
> And can someone take a look at config of code obfuscator used for linting?
> It spits out amount of information showing that noone looked at updating it
> to current version:

You're supposed to use the edk2 uncrustify fork that you get *somehow*
(no, I don't remember how, I do remember it was a pain though).
But seriously, speaking as someone with a completely different
personal coding style, the coding style really isn't all that bad (at
the very least, it's tolerable), it's just a bastardized NT-kernel
style, and with uncrustify you can auto-format all your code.

What really pains me is CRLF vs LF, which breaks *every git tool*.
Seriously, applying patches tends to be a pain, at least on Linux.

-- 
Pedro


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

Re: [edk2-devel] Maybe it is time to update code style?

[Marcin Juszkiewicz]

W dniu 25.09.2023 o 20:03, Pedro Falcato pisze:
> On Mon, Sep 25, 2023 at 10:03 AM Marcin Juszkiewicz

>> And can someone take a look at config of code obfuscator used for linting?
>> It spits out amount of information showing that noone looked at updating it
>> to current version:
> 
> You're supposed to use the edk2 uncrustify fork that you get *somehow*
> (no, I don't remember how, I do remember it was a pain though).

If it is not latest upstream then maybe it should be merged into 
BaseTools and kept there?

> But seriously, speaking as someone with a completely different
> personal coding style, the coding style really isn't all that bad (at
> the very least, it's tolerable), it's just a bastardized NT-kernel
> style, and with uncrustify you can auto-format all your code.

When it works and do not complain about own config.

> What really pains me is CRLF vs LF, which breaks *every git tool*.
> Seriously, applying patches tends to be a pain, at least on Linux.

Argh. That's pain in the ass. I handle most of (outside of ML) sharing 
patches via own fork repo as it is easier than git format-patch+am combo.



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