If you're curious, you can look at my Ext4Pkg ( https://github.com/tianocore/edk2-platforms/tree/master/Features/Ext4Pkg/Ext4Dxe) to look at the effects of align_func_params_gap and align_func_params_span = 2. Things look okay and similar to other edk2-styled code. Best regards, Pedro On Tue, Nov 16, 2021 at 11:54 PM Michael Kubacki < mikuback@linux.microsoft.com> wrote: > The poc_5 branch config file is based on the file I'm pushing to the > Uncrustify fork soon: > > https://dev.azure.com/projectmu/Uncrustify/_git/Uncrustify/pullrequest/24?_a=files&path=/etc/edk2.cfg > > So they're already being used and the file in the poc_5 branch was of > course used to generate the results in the branch. > > Note that I do have "align_func_params" set to true. I believe I saw > some occasional anomalies in the past when I set > "align_func_params_span" and the gap to larger values but I don't recall > exactly what they were. I'd be happy to try that again soon though. > > On 11/16/2021 6:30 PM, Pedro Falcato wrote: > > 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 > > > > 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 > > < > 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 > > > > > > >. > > > There's a bit of diff noise due to it being based on > > > > > 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 > > > > > > > > > < > 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 > > > > > > > >> 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 > > > > > >> > > > >> Sent: Tuesday, November 16, 2021 10:54 AM > > > >> To: devel@edk2.groups.io > > >; Kinney, > > > Michael D > > > > > >> > > > >> 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 > > > > > > > > > >> On > > Behalf Of > > > Michael Kubacki > > > >>>> Sent: Tuesday, November 16, 2021 10:25 AM > > > >>>> To: devel@edk2.groups.io > > >; > > > Kinney, Michael D > > > > > >> > > > >>>> 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> > > > > > < > 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 > > > > > > >), 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> > > > > > < > 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 > > > > > > > < > 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 > -- Pedro Falcato