<div dir="ltr"><div>Yes, I do.</div><div> </div><div>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.</div><div>"force" instead of "add" for sp_before_sparen is a good idea though.</div><div>I assume these are good additions to the "upstream" edk2.cfg? </div></div> <div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Nov 16, 2021 at 11:20 PM Michael Kubacki <<a href="mailto:mikuback@linux.microsoft.com">mikuback@linux.microsoft.com</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Pedro, By "new config", do you mean the config file in the latest branch <a href="https://github.com/makubacki/edk2/blob/uncrustify_poc_5/uncrustify.cfg" rel="noreferrer" target="_blank">https://github.com/makubacki/edk2/blob/uncrustify_poc_5/uncrustify.cfg</a>)? 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: > <a href="https://gist.github.com/heatd/5bb048a188726cb8bc9f5b4a844b0ec2" rel="noreferrer" target="_blank">https://gist.github.com/heatd/5bb048a188726cb8bc9f5b4a844b0ec2</a> > <<a href="https://gist.github.com/heatd/5bb048a188726cb8bc9f5b4a844b0ec2" rel="noreferrer" target="_blank">https://gist.github.com/heatd/5bb048a188726cb8bc9f5b4a844b0ec2</a>>. > There's a bit of diff noise due to it being based on > <a href="https://dev.azure.com/projectmu/_git/Uncrustify?path=/etc/edk2.cfg" rel="noreferrer" target="_blank">https://dev.azure.com/projectmu/_git/Uncrustify?path=/etc/edk2.cfg</a> > <<a href="https://dev.azure.com/projectmu/_git/Uncrustify?path=/etc/edk2.cfg" rel="noreferrer" target="_blank">https://dev.azure.com/projectmu/_git/Uncrustify?path=/etc/edk2.cfg</a>> but > diffed on > <a href="https://github.com/makubacki/edk2/blob/uncrustify_poc_2/.uncrustify/edk2.cfg" rel="noreferrer" target="_blank">https://github.com/makubacki/edk2/blob/uncrustify_poc_2/.uncrustify/edk2.cfg</a> > <<a href="https://github.com/makubacki/edk2/blob/uncrustify_poc_2/.uncrustify/edk2.cfg" rel="noreferrer" target="_blank">https://github.com/makubacki/edk2/blob/uncrustify_poc_2/.uncrustify/edk2.cfg</a>>. > > Thanks, > Pedro > > On Tue, Nov 16, 2021 at 7:26 PM Michael Kubacki > <<a href="mailto:mikuback@linux.microsoft.com" target="_blank">mikuback@linux.microsoft.com</a> <mailto:<a href="mailto:mikuback@linux.microsoft.com" target="_blank">mikuback@linux.microsoft.com</a>>> 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 <<a href="mailto:mikuback@linux.microsoft.com" target="_blank">mikuback@linux.microsoft.com</a> > <mailto:<a href="mailto:mikuback@linux.microsoft.com" target="_blank">mikuback@linux.microsoft.com</a>>> > >> Sent: Tuesday, November 16, 2021 10:54 AM > >> To: <a href="mailto:devel@edk2.groups.io" target="_blank">devel@edk2.groups.io</a> <mailto:<a href="mailto:devel@edk2.groups.io" target="_blank">devel@edk2.groups.io</a>>; Kinney, > Michael D <<a href="mailto:michael.d.kinney@intel.com" target="_blank">michael.d.kinney@intel.com</a> > <mailto:<a href="mailto:michael.d.kinney@intel.com" target="_blank">michael.d.kinney@intel.com</a>>> > >> 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: <a href="mailto:devel@edk2.groups.io" target="_blank">devel@edk2.groups.io</a> <mailto:<a href="mailto:devel@edk2.groups.io" target="_blank">devel@edk2.groups.io</a>> > <<a href="mailto:devel@edk2.groups.io" target="_blank">devel@edk2.groups.io</a> <mailto:<a href="mailto:devel@edk2.groups.io" target="_blank">devel@edk2.groups.io</a>>> On Behalf Of > Michael Kubacki > >>>> Sent: Tuesday, November 16, 2021 10:25 AM > >>>> To: <a href="mailto:devel@edk2.groups.io" target="_blank">devel@edk2.groups.io</a> <mailto:<a href="mailto:devel@edk2.groups.io" target="_blank">devel@edk2.groups.io</a>>; > Kinney, Michael D <<a href="mailto:michael.d.kinney@intel.com" target="_blank">michael.d.kinney@intel.com</a> > <mailto:<a href="mailto:michael.d.kinney@intel.com" target="_blank">michael.d.kinney@intel.com</a>>> > >>>> 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 > >>>> > <a href="https://github.com/makubacki/edk2/commits/uncrustify_poc_3_with_headers" rel="noreferrer" target="_blank">https://github.com/makubacki/edk2/commits/uncrustify_poc_3_with_headers</a> > <<a href="https://github.com/makubacki/edk2/commits/uncrustify_poc_3_with_headers" rel="noreferrer" target="_blank">https://github.com/makubacki/edk2/commits/uncrustify_poc_3_with_headers</a>>), > 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 > (<a href="https://github.com/makubacki/edk2/tree/uncrustify_poc_5" rel="noreferrer" target="_blank">https://github.com/makubacki/edk2/tree/uncrustify_poc_5</a> > <<a href="https://github.com/makubacki/edk2/tree/uncrustify_poc_5" rel="noreferrer" target="_blank">https://github.com/makubacki/edk2/tree/uncrustify_poc_5</a>>), I see the > >>>>> uncrustify.cfg configuration file in the root. > >>>>> > >>>>> > <a href="https://github.com/makubacki/edk2/blob/uncrustify_poc_5/uncrustify.cfg" rel="noreferrer" target="_blank">https://github.com/makubacki/edk2/blob/uncrustify_poc_5/uncrustify.cfg</a> > <<a href="https://github.com/makubacki/edk2/blob/uncrustify_poc_5/uncrustify.cfg" rel="noreferrer" target="_blank">https://github.com/makubacki/edk2/blob/uncrustify_poc_5/uncrustify.cfg</a>> > >>>>> > >>>>> However, in your Wiki, you provide examples where this > configuration file is in an > >>>>> .uncrustify directory > >>>>> > >>>>> > <a href="https://dev.azure.com/projectmu/Uncrustify/_wiki/wikis/Uncrustify.wiki/1/Project-Mu-(EDK-II)-Fork-Readme" rel="noreferrer" target="_blank">https://dev.azure.com/projectmu/Uncrustify/_wiki/wikis/Uncrustify.wiki/1/Project-Mu-(EDK-II)-Fork-Readme</a> > <<a href="https://dev.azure.com/projectmu/Uncrustify/_wiki/wikis/Uncrustify.wiki/1/Project-Mu-(EDK-II)-Fork-Readme" rel="noreferrer" target="_blank">https://dev.azure.com/projectmu/Uncrustify/_wiki/wikis/Uncrustify.wiki/1/Project-Mu-(EDK-II)-Fork-Readme</a>> > >>>>> > >>>>> 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 </blockquote></div> -- <div dir="ltr" class="gmail_signature"><div dir="ltr">Pedro Falcato</div></div> <div width="1" style="color:white;clear:both">_._,_._,_</div> <hr> Groups.io Links: You receive all messages sent to this group. <a target="_blank" href="https://edk2.groups.io/g/devel/message/83796">View/Reply Online (#83796)</a> | | <a target="_blank" href="https://groups.io/mt/87100207/1813853">Mute This Topic</a> | <a href="https://edk2.groups.io/g/devel/post">New Topic</a> <a href="https://edk2.groups.io/g/devel/editsub/1813853">Your Subscription</a> | <a href="mailto:devel+owner@edk2.groups.io">Contact Group Owner</a> | <a href="https://edk2.groups.io/g/devel/unsub">Unsubscribe</a> [edk2-devel-archive@redhat.com] <div width="1" style="color:white;clear:both">_._,_._,_</div>