Re: [edk2-devel] Uncrustify configuration file and file/function templates

["Pedro Falcato" <pedro.falcato@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 7947 bytes --]

Yes, I do.

Essentially, my changes make it so the function parameters are aligned and
there are always two spaces between the parameter's type and name; it also
adds spaces between control statements, as you mentioned.
"force" instead of "add" for sp_before_sparen is a good idea though.
I assume these are good additions to the "upstream" edk2.cfg?

On Tue, Nov 16, 2021 at 11:20 PM Michael Kubacki <
mikuback@linux.microsoft.com> wrote:

> Hi Pedro,
>
> By "new config", do you mean the config file in the latest branch
> https://github.com/makubacki/edk2/blob/uncrustify_poc_5/uncrustify.cfg)?
>
> The main difference I see from the diff in that gist and poc_5 is that
> "align_func_params_gap" and "align_func_params_span" are two instead of
> zero.
>
> The value "force" instead of "add" for "sp_before_sparen" will ensure a
> single space is always added between a control statement and the
> parenthesis.
>
> Thanks,
> Michael
>
> On 11/16/2021 6:05 PM, Pedro Falcato wrote:
> > Hi,
> >
> > RE: Uncrustify's configuration
> >
> > I had made a few changes to the config file, when trying out Uncrustify,
> > a few months ago, on Mike Kinney's suggestion. In my experience, the new
> > config file reflects the edk2 coding style better.
> >
> > In particular, it adds spaces between things like 'if' and the
> > parenthesis, and better aligns functions' parameters.
> > Feel free to check out the diff:
> > https://gist.github.com/heatd/5bb048a188726cb8bc9f5b4a844b0ec2
> > <https://gist.github.com/heatd/5bb048a188726cb8bc9f5b4a844b0ec2>.
> > There's a bit of diff noise due to it being based on
> > https://dev.azure.com/projectmu/_git/Uncrustify?path=/etc/edk2.cfg
> > <https://dev.azure.com/projectmu/_git/Uncrustify?path=/etc/edk2.cfg>
> but
> > diffed on
> >
> https://github.com/makubacki/edk2/blob/uncrustify_poc_2/.uncrustify/edk2.cfg
> > <
> https://github.com/makubacki/edk2/blob/uncrustify_poc_2/.uncrustify/edk2.cfg
> >.
> >
> > Thanks,
> > Pedro
> >
> > On Tue, Nov 16, 2021 at 7:26 PM Michael Kubacki
> > <mikuback@linux.microsoft.com <mailto:mikuback@linux.microsoft.com>>
> wrote:
> >
> >     I think that makes sense. I will look into it further and let you
> know
> >     if there's any downsides found.
> >
> >     On 11/16/2021 2:18 PM, Kinney, Michael D wrote:
> >      > Could we add this feature to the Uncrustify CI Plugin?
> >      >
> >      > Mike
> >      >
> >      >> -----Original Message-----
> >      >> From: Michael Kubacki <mikuback@linux.microsoft.com
> >     <mailto:mikuback@linux.microsoft.com>>
> >      >> Sent: Tuesday, November 16, 2021 10:54 AM
> >      >> To: devel@edk2.groups.io <mailto:devel@edk2.groups.io>; Kinney,
> >     Michael D <michael.d.kinney@intel.com
> >     <mailto:michael.d.kinney@intel.com>>
> >      >> Subject: Re: [edk2-devel] Uncrustify configuration file and
> >     file/function templates
> >      >>
> >      >> I would prefer to have a single version of the file if possible
> to
> >      >> reduce synchronization issues across the two copies.
> >      >>
> >      >> It seems that a CI plugin to read the contents of the template
> >     files and
> >      >> search incoming code for that text wouldn't be too difficult to
> >     add as a
> >      >> new plugin.
> >      >>
> >      >> Thanks,
> >      >> Michael
> >      >>
> >      >> On 11/16/2021 1:31 PM, Michael D Kinney wrote:
> >      >>> Hi Michael,
> >      >>>
> >      >>> Should we have 2 versions of the config file?
> >      >>>
> >      >>> One used by automation tools such as CI and git hooks that do
> >     not use the
> >      >>> templates.
> >      >>>
> >      >>> And another one that a developer can optionally use that will
> >     add the
> >      >>> templates for missing file/function headers that the developer
> >     then needs
> >      >>> to fill out.
> >      >>>
> >      >>> One concern I have about the templates is if they get used but
> >     a developer
> >      >>> does not fill in the missing information.  It would be best if
> >     a CI check
> >      >>> rejects a file/function header that has not been filled in.
> >      >>>
> >      >>> Mike
> >      >>>
> >      >>>> -----Original Message-----
> >      >>>> From: devel@edk2.groups.io <mailto:devel@edk2.groups.io>
> >     <devel@edk2.groups.io <mailto:devel@edk2.groups.io>> On Behalf Of
> >     Michael Kubacki
> >      >>>> Sent: Tuesday, November 16, 2021 10:25 AM
> >      >>>> To: devel@edk2.groups.io <mailto:devel@edk2.groups.io>;
> >     Kinney, Michael D <michael.d.kinney@intel.com
> >     <mailto:michael.d.kinney@intel.com>>
> >      >>>> Subject: Re: [edk2-devel] Uncrustify configuration file and
> >     file/function templates
> >      >>>>
> >      >>>> Hi Mike,
> >      >>>>
> >      >>>> Those were just disabled because I typically run a separate
> >     invocation
> >      >>>> of Uncrustify with them enabled to isolate code which is
> missing
> >      >>>> file/function headers. My thought was the templates are
> >     helpful but we
> >      >>>> would need to individually identify where they are placed to
> >     file TCBZs
> >      >>>> for maintainers to replace the template with the actual
> >     information.
> >      >>>>
> >      >>>> In some of my previous poc branches (like
> >      >>>>
> >
> https://github.com/makubacki/edk2/commits/uncrustify_poc_3_with_headers
> >     <
> https://github.com/makubacki/edk2/commits/uncrustify_poc_3_with_headers>),
> >     I
> >      >>>> also pushed a branch with those results.
> >      >>>>
> >      >>>> So I do think we would want them enabled in the final config
> >     file. We
> >      >>>> can also review the contents of the templates in the future
> >     patch series
> >      >>>> to see if any changes are recommended.
> >      >>>>
> >      >>>> I prefer using a .uncrustify directory to help group related
> >     collateral
> >      >>>> but I don't have a strong opinion there.
> >      >>>>
> >      >>>> Thanks,
> >      >>>> Michael
> >      >>>>
> >      >>>> On 11/16/2021 12:16 PM, Michael D Kinney wrote:
> >      >>>>> Hi Michael,
> >      >>>>>
> >      >>>>> In your POC branch
> >     (https://github.com/makubacki/edk2/tree/uncrustify_poc_5
> >     <https://github.com/makubacki/edk2/tree/uncrustify_poc_5>), I see
> the
> >      >>>>> uncrustify.cfg configuration file in the root.
> >      >>>>>
> >      >>>>>
> >
> https://github.com/makubacki/edk2/blob/uncrustify_poc_5/uncrustify.cfg
> >     <
> https://github.com/makubacki/edk2/blob/uncrustify_poc_5/uncrustify.cfg>
> >      >>>>>
> >      >>>>> However, in your Wiki, you provide examples where this
> >     configuration file is in an
> >      >>>>> .uncrustify directory
> >      >>>>>
> >      >>>>>
> >
> https://dev.azure.com/projectmu/Uncrustify/_wiki/wikis/Uncrustify.wiki/1/Project-Mu-(EDK-II)-Fork-Readme
> >     <
> https://dev.azure.com/projectmu/Uncrustify/_wiki/wikis/Uncrustify.wiki/1/Project-Mu-(EDK-II)-Fork-Readme
> >
> >      >>>>>
> >      >>>>> The uncrustify.cfg files also contains commented out settings
> >     for the file header
> >      >>>>> and function header templates.
> >      >>>>>
> >      >>>>>        # cmt_insert_file_header          =
> >     default_file_header.txt
> >      >>>>>        # cmt_insert_func_header          =
> >     default_function_header.txt
> >      >>>>>
> >      >>>>> Are these disabled on purpose?
> >      >>>>>
> >      >>>>> Do we want to enable them?  If so, should the uncrustify
> >     configuration file
> >      >>>>> and the templates go into a .uncrustify directory?
> >      >>>>>
> >      >>>>> Thanks,
> >      >>>>>
> >      >>>>> Mike
> >      >>>>>
> >      >>>>>
> >      >>>>>
> >      >>>>>
> >      >>>>>
> >      >>>>>
> >      >>>>>
> >      >>>>
> >      >>>>
> >      >>>>
> >      >>>>
> >      >>>
> >      >>>
> >      >>>
> >      >>>
> >      >>>
> >      >>>
> >
> >
> >
> >
> >
> >
> >
> > --
> > Pedro Falcato
> > 
>


-- 
Pedro Falcato

[-- Attachment #2: Type: text/html, Size: 14260 bytes --]