Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed arguments

["Michael Kubacki" <mikuback@linux.microsoft.com>]

I'm catching up on this thread so let me know if I miss something.

Uncrustify can perform conversions/enforcements like adjusting code to < 
80 columns.

The edk2 uncrustify configuration file is here and you will see that I 
commented column width enforcement:

https://github.com/tianocore/edk2/blob/master/.pytool/Plugin/UncrustifyCheck/uncrustify.cfg#L19.

Uncrustify aside, column width is particularly difficult to adjust 
consistently. Both humans and tools have to make many (constant) 
situational decisions based on code structure.

However, it should be possible. I've taken the current uncrustify 
configuration file, made minimal changes to restrict code to 80 columns, 
and published the results based on the current edk2 code tree here:

https://github.com/makubacki/edk2/tree/uncrustify_80_column_change

You can see the I ran uncrustify 3 times to reach a mostly steady state. 
The issue after the second run is that uncrustify is confused about what 
to do with single macros that exceed 80 columns.

Examples:
1. 
https://github.com/makubacki/edk2/blob/uncrustify_80_column_change/MdePkg/Include/IndustryStandard/Acpi30.h#L702-#L704
2. 
https://github.com/makubacki/edk2/blob/uncrustify_80_column_change/MdePkg/Include/IndustryStandard/IpmiNetFnApp.h#L1023-#L1031

There are other cases there as well.

Anyway, if those were reduced in length, I think we could reach a steady 
state. Some other minor tweaks might also be required.

Regarding function calls, I put together the following branch to 
demonstrate some examples of what is done now.

In summary, multiple arguments can be provided on a single line (with no 
width or argument count maximum) or multiple lines. If a single argument 
is multi-line, then all arguments must be on a unique line to follow the 
multi-line argument convention.

See the top two commits in this branch for examples:

https://github.com/makubacki/edk2/tree/func_arg_formatting_demo

I agree uncrustify and the spec be in sync.

Regards,
Michael

On 11/14/2022 12:07 PM, Michael D Kinney wrote:
> I disagree that they can coexist.
> 
> If uncrustify is forcing 1 arg per line, then a developer that follows a 
> CSS that allows multiple per line, the code change will be rejected by 
> EDK II CI.
> 
> The CSS and Uncristify behavior need to be aligned.If we want a CSS 
> change that requires Uncristify changes, then they have to be coordinated.
> 
> Mike
> 
> *From:*devel@edk2.groups.io <devel@edk2.groups.io> *On Behalf Of *Chang, 
> Abner via groups.io
> *Sent:* Sunday, November 13, 2022 5:10 PM
> *To:* devel@edk2.groups.io; Kinney, Michael D 
> <michael.d.kinney@intel.com>; Laszlo Ersek <lersek@redhat.com>; Kubacki, 
> Michael <michael.kubacki@microsoft.com>
> *Subject:* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 
> 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed 
> arguments
> 
> [AMD Official Use Only - General]
> 
> For this case, we don’t have to take another global reformatting. These 
> two formats can coexisting without the conflict.  We just allow the 
> condense argus format in CSS. Also, update Uncrustify to not forcing 
> each argument at its own line.
> 
> The current Uncrustify behavior seems to me match the CCS spec. But this 
> patch was sent to allow the multiple argus at the same line, which was 
> not proposed to fix the issue in current Uncrustify. You sure we just 
> close this issue?
> 
> Abner
> 
> *From:* devel@edk2.groups.io <mailto:devel@edk2.groups.io> 
> <devel@edk2.groups.io <mailto:devel@edk2.groups.io>> *On Behalf Of 
> *Michael D Kinney via groups.io
> *Sent:* Monday, November 14, 2022 1:36 AM
> *To:* devel@edk2.groups.io <mailto:devel@edk2.groups.io>; Chang, Abner 
> <Abner.Chang@amd.com <mailto:Abner.Chang@amd.com>>; Laszlo Ersek 
> <lersek@redhat.com <mailto:lersek@redhat.com>>; Kubacki, Michael 
> <michael.kubacki@microsoft.com <mailto:michael.kubacki@microsoft.com>>; 
> Kinney, Michael D <michael.d.kinney@intel.com 
> <mailto:michael.d.kinney@intel.com>>
> *Subject:* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 
> 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed 
> arguments
> 
> 	
> 
> *Caution:*This message originated from an External Source. Use proper 
> caution when opening attachments, clicking links, or responding.
> 
> We do not want another global format change because that make git blame 
> difficult to use.
> 
> Are any clarifications required to describe the current Uncrustify 
> behavior?  Or is the description correct?
> 
> If the current description matches Uncristify behavior, then I recommend 
> we close this issue as will not fix.
> 
> Mike
> 
> *From:* devel@edk2.groups.io <mailto:devel@edk2.groups.io> 
> <devel@edk2.groups.io <mailto:devel@edk2.groups.io>> *On Behalf Of 
> *Chang, Abner via groups.io
> *Sent:* Sunday, November 13, 2022 12:45 AM
> *To:* Kinney, Michael D <michael.d.kinney@intel.com 
> <mailto:michael.d.kinney@intel.com>>; devel@edk2.groups.io 
> <mailto:devel@edk2.groups.io>; Laszlo Ersek <lersek@redhat.com 
> <mailto:lersek@redhat.com>>; Kubacki, Michael 
> <michael.kubacki@microsoft.com <mailto:michael.kubacki@microsoft.com>>
> *Subject:* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 
> 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed 
> arguments
> 
> [AMD Official Use Only - General]
> 
> Uncrustify can fix the first argument that is not at the indent with two 
> space. It also can fix the first argument that is not at the new line.
> 
> But it also makes each argument a new line if multiple args are 
> condensed in one line. That is what we have to update Uncrustify if we 
> have this patch merged to CCS.
> 
> +Michael Kubacki in loop.
> 
> Abner
> 
> *From:* Kinney, Michael D <michael.d.kinney@intel.com 
> <mailto:michael.d.kinney@intel.com>>
> *Sent:* Sunday, November 13, 2022 9:58 AM
> *To:* devel@edk2.groups.io <mailto:devel@edk2.groups.io>; Chang, Abner 
> <Abner.Chang@amd.com <mailto:Abner.Chang@amd.com>>; Laszlo Ersek 
> <lersek@redhat.com <mailto:lersek@redhat.com>>; Kinney, Michael D 
> <michael.d.kinney@intel.com <mailto:michael.d.kinney@intel.com>>
> *Subject:* RE: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 
> 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed 
> arguments
> 
> 	
> 
> *Caution:*This message originated from an External Source. Use proper 
> caution when opening attachments, clicking links, or responding.
> 
> Is this exactly what Uncrustify does now?
> 
> Mike
> 
> *From:* devel@edk2.groups.io <mailto:devel@edk2.groups.io> 
> <devel@edk2.groups.io <mailto:devel@edk2.groups.io>> *On Behalf Of 
> *Chang, Abner via groups.io
> *Sent:* Saturday, November 12, 2022 5:36 PM
> *To:* Laszlo Ersek <lersek@redhat.com <mailto:lersek@redhat.com>>; 
> devel@edk2.groups.io <mailto:devel@edk2.groups.io>
> *Subject:* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 
> 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed 
> arguments
> 
> Hi all,
> As we are going to release CCS 2.3, we would like to address some 
> pending issues of CCS. For this, I think we can,
> - Still keep the one line per argument style in CCS although the 
> multi-arguments in the one line style can cover this. This avoids 
> confusion from readers and questions about if they can do the one-line 
> per argument style.
> - If the arguments are in different lines, the first argument must be 
> indented with two spaces from the start of the function name or the 
> member function name.
> How is this?
> 
> Abner
> 
>