Re: [Xen-devel] [RFC] xen: Add .astylerc for automated style-formatting

[Tamas K Lengyel <tamas@xxxxxxxxxxxxx>]

On Thu, Jul 18, 2019 at 9:02 AM Julien Grall <julien.grall@xxxxxxx> wrote:
>
> Hi Tamas,
>
> Adding Lars, Artem and Iurii. Iurii has been working on a version for
> clang-format recently.
>
> On 7/18/19 3:43 PM, Tamas K Lengyel wrote:
> > Using astyle (http://astyle.sourceforge.net) can greatly reduce the 
> > overhead of
> > manually checking and applying style-fixes to source-code. The included
> > .astylerc is the closest approximation of the established Xen style 
> > (including
> > styles not formally spelled out by CODING_STYLE but commonly requested).
> >
> > Checking the comment styles are not included in the automation.
> >
> > Incorporating Xen's exception to the do-while style is only partially 
> > possible,
> > thus a change is proposed to the CODING_STYLE of moving the brace from "do 
> > {"
> > to the next line.
> >
> > Most of Xen's code-base is non-conforming at the moment: 289 files pass
> > unchanged, 876 have some style issues.
> >
> > Ideally we can slowly migrate the entire code-base to be conforming, thus
> > eliminating the need of discussing and enforcing style issues manually on 
> > the
> > mailinglist.
>
> I quite like the idea of an automatic coding style checker. However, it
> is a bit concerning that not even a 1/3 of the files are able to pass
> the coding style you suggest. Could you explain whether this is because
> the files does not already follow Xen coding style or is it just the
> difference with astyle?
>
> What are the main style issues?

Looks like a lot of files that don't follow the Xen coding style
as-is. Alignment issues seem to me to be the most common errors. See
the full diff here:

https://gist.github.com/tklengyel/c5cac14a0d57f119dd7747a1be6fb260

We can perhaps tune some aspects of it we disagree with the astyle
generated style and try to override it. I did my best to make it
conform to the existing Xen style but certainly there could be other
tweaks made to reduce the churn.

> >
> > Signed-off-by: Tamas K Lengyel <tamas@xxxxxxxxxxxxx>
> > ---
> >   .astylerc    | 14 ++++++++++++++
> >   CODING_STYLE | 18 +++++++++++++++---
> >   2 files changed, 29 insertions(+), 3 deletions(-)
> >   create mode 100644 .astylerc
> >
> > diff --git a/.astylerc b/.astylerc
> > new file mode 100644
> > index 0000000000..bbd1d55ddd
> > --- /dev/null
> > +++ b/.astylerc
> > @@ -0,0 +1,14 @@
> > +style=bsd
> > +suffix=none
> > +align-pointer=name
> > +align-reference=name
> > +indent=spaces=4
> > +max-code-length=80
> > +min-conditional-indent=0
> > +attach-closing-while
> > +remove-braces
> > +indent-switches
> > +break-one-line-headers
> > +keep-one-line-blocks
> > +pad-comma
> > +pad-header
> > diff --git a/CODING_STYLE b/CODING_STYLE
> > index 6cc5b774cf..0b37f7ae4d 100644
> > --- a/CODING_STYLE
> > +++ b/CODING_STYLE
> > @@ -60,8 +60,8 @@ Bracing
> >   -------
> >
> >   Braces ('{' and '}') are usually placed on a line of their own, except
> > -for the do/while loop.  This is unlike the Linux coding style and
> > -unlike K&R.  do/while loops are an exception. e.g.:
> > +for the while-part of do/while loops.  This is unlike the Linux coding 
> > style
> > +and unlike K&R.  do/while loops are an exception. e.g.:
> >
> >   if ( condition )
> >   {
> > @@ -77,7 +77,8 @@ while ( condition )
> >       /* Do stuff. */
> >   }
> >
> > -do {
> > +do
> > +{
> >       /* Do stuff. */
> >   } while ( condition );
> >
> > @@ -120,3 +121,14 @@ the end of files.  It should be:
> >    * indent-tabs-mode: nil
> >    * End:
> >    */
> > +
> > +Automated style formatting using astyle
> > +---------------------------------------
> > +
> > +The .astylerc included in the Xen tree incorporates most of Xen's
> > +style requirements, except the formatting of comments.
> > +
> > +The steps to automatically format a file are:
> > +
> > +export ARTISTIC_STYLE_OPTIONS=".astylerc"
> > +astyle <source or header file>
>
> I think you want to provide easy way for the user to install/compile it.
> So there are an higher chance for them to use it.

It is generally included in all major distributions, I just do apt-get
install astyle on debian/ubuntu.

>
> Long-term we probably want to get this hooked to the CI loop.

Indeed. I already do that in my project's CI using Travis and it has
been awesome (https://github.com/tklengyel/drakvuf/blob/master/.travis.yml#L28).

Tamas

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel