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 < mikub...@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 > > <mikub...@linux.microsoft.com <mailto:mikub...@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 <mikub...@linux.microsoft.com > > <mailto:mikub...@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.kin...@intel.com > > <mailto:michael.d.kin...@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.kin...@intel.com > > <mailto:michael.d.kin...@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 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#83796): https://edk2.groups.io/g/devel/message/83796 Mute This Topic: https://groups.io/mt/87100207/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
