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

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

Have these changes been discussed in a community forum? Do you think we 
could talk about it in the Tianocore Tools & CI Meeting today?

Thanks,
Michael

On 11/14/2022 12:37 PM, Michael Kubacki wrote:
> 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
>>
>>
> 
> 
> 
>