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

["Michael D Kinney" <michael.d.kinney@intel.com>]

Hi Michael,

Yes. We can discuss in Tools/CI meeting later today.  Not just these specific issues
but also discuss process to coordinate changes to both CSS and Uncrustify config.

My recommendation is to close these older issues as "will not fix".  The review
of those topics 5 years were not completely resolved.  We have since added uncrustify
with CI checks.  If there are some elements of these previous topics that the 
community still thinks are valuable to consider, then those can be opened in a new
BZ and use the process that is established to coordinate the CSS and uncrustify
changes. 

Thanks,

Mike

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael Kubacki
> Sent: Monday, November 14, 2022 10:49 AM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io; abner.chang@amd.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
> 
> I saw those, but they're from over 5 years ago and the community has
> changed since then.
> 
> This is an impactful change. It will impact every line of code written.
> 
> I'm not suggesting this go to the Tools & CI Meeting because of
> uncrustify. I'm not concerned with any impact to uncrustify.
> 
> This came to my attention because I was added to thread. I would like to
> suggest that we raise more awareness about this change in a meeting with
> members of the community to capture additional feedback.
> 
> Also, the logistics (uncrustify aside) about how to execute this change
> are unclear (as written in this thread and the BZ) and it would be
> easier/faster to discuss in a community call.
> 
> Is that possible?
> 
> Thanks,
> Michael
> 
> On 11/14/2022 1:25 PM, Kinney, Michael D wrote:
> > Hi Michael,
> >
> > Yes.  They have been discussed.  There were patches and discussion on mailing
> > lists back in 2017.  Long before enabling uncrustify. I provided links to
> > these conversations in the following email about a week ago.
> >
> > https://edk2.groups.io/g/devel/message/96038
> >
> >
> > Mike
> >
> >> -----Original Message-----
> >> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael Kubacki
> >> Sent: Monday, November 14, 2022 10:05 AM
> >> To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>; abner.chang@amd.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
> >>
> >> 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
> >>>>
> >>>>
> >>>
> >>>
> >>>
> >>>
> >>
> >>
> >>
> >>
> >
> 
> 
> 
>