[edk2-devel] [edk2-CCodingStandardsSpecification PATCH 2/2] Source Files / Spacing / Multi-line func. calls: allow condensed arguments
Michael Kubacki
mikuback at linux.microsoft.com
Mon Nov 14 18:05:28 UTC 2022
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 at edk2.groups.io <devel at edk2.groups.io> *On Behalf Of
>> *Chang, Abner via groups.io
>> *Sent:* Sunday, November 13, 2022 5:10 PM
>> *To:* devel at edk2.groups.io; Kinney, Michael D
>> <michael.d.kinney at intel.com>; Laszlo Ersek <lersek at redhat.com>;
>> Kubacki, Michael <michael.kubacki at 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 at edk2.groups.io <mailto:devel at edk2.groups.io>
>> <devel at edk2.groups.io <mailto:devel at edk2.groups.io>> *On Behalf Of
>> *Michael D Kinney via groups.io
>> *Sent:* Monday, November 14, 2022 1:36 AM
>> *To:* devel at edk2.groups.io <mailto:devel at edk2.groups.io>; Chang, Abner
>> <Abner.Chang at amd.com <mailto:Abner.Chang at amd.com>>; Laszlo Ersek
>> <lersek at redhat.com <mailto:lersek at redhat.com>>; Kubacki, Michael
>> <michael.kubacki at microsoft.com
>> <mailto:michael.kubacki at microsoft.com>>; Kinney, Michael D
>> <michael.d.kinney at intel.com <mailto:michael.d.kinney at 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 at edk2.groups.io <mailto:devel at edk2.groups.io>
>> <devel at edk2.groups.io <mailto:devel at 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 at intel.com
>> <mailto:michael.d.kinney at intel.com>>; devel at edk2.groups.io
>> <mailto:devel at edk2.groups.io>; Laszlo Ersek <lersek at redhat.com
>> <mailto:lersek at redhat.com>>; Kubacki, Michael
>> <michael.kubacki at microsoft.com <mailto:michael.kubacki at 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 at intel.com
>> <mailto:michael.d.kinney at intel.com>>
>> *Sent:* Sunday, November 13, 2022 9:58 AM
>> *To:* devel at edk2.groups.io <mailto:devel at edk2.groups.io>; Chang, Abner
>> <Abner.Chang at amd.com <mailto:Abner.Chang at amd.com>>; Laszlo Ersek
>> <lersek at redhat.com <mailto:lersek at redhat.com>>; Kinney, Michael D
>> <michael.d.kinney at intel.com <mailto:michael.d.kinney at 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 at edk2.groups.io <mailto:devel at edk2.groups.io>
>> <devel at edk2.groups.io <mailto:devel at edk2.groups.io>> *On Behalf Of
>> *Chang, Abner via groups.io
>> *Sent:* Saturday, November 12, 2022 5:36 PM
>> *To:* Laszlo Ersek <lersek at redhat.com <mailto:lersek at redhat.com>>;
>> devel at edk2.groups.io <mailto:devel at 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
>>
>>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#96354): https://edk2.groups.io/g/devel/message/96354
Mute This Topic: https://groups.io/mt/30907266/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list