Add a tentative .clang-format for LilyPond. (issue 561340043 by hanwenn@gmail.com)

classic Classic list List threaded Threaded
15 messages Options
Reply | Threaded
Open this post in threaded view
|

Add a tentative .clang-format for LilyPond. (issue 561340043 by hanwenn@gmail.com)

Dev mailing list
For which clang-format version is this format file?  It doesn't work
with 7.0.1, for example.  Please add this information to the file.

https://codereview.appspot.com/561340043/

Reply | Threaded
Open this post in threaded view
|

Re: Add a tentative .clang-format for LilyPond. (issue 561340043 by hanwenn@gmail.com)

Han-Wen Nienhuys-3
Reviewers: lemzwerg,

Message:
On 2020/01/27 10:07:20, lemzwerg wrote:
> For which clang-format version is this format file?  It doesn't work
with 7.0.1,
> for example.  Please add this information to the file.

$ clang-format --version
clang-format version 9.0.0 (Fedora 9.0.0-1.fc31)

can you pinpoint which statements cause problems? We could leave out the
default values for enhanced compatibility.

Description:
Add a tentative .clang-format for LilyPond.

To try it out, run

  clang-format -i lily/include/*[ch] lily/*.cc

Please review this at https://codereview.appspot.com/561340043/

Affected files (+128, -0 lines):
  A .clang-format



Reply | Threaded
Open this post in threaded view
|

Re: Add a tentative .clang-format for LilyPond. (issue 561340043 by hanwenn@gmail.com)

Dev mailing list
In reply to this post by Dev mailing list
> just the overrides

The new version works just fine.  Shall I still inspect the previous
version for problematic statements?

The output looks good.  BTW, for the sake of Emacs, it would be nice if
two spaces after a full dot could be retained in comments.  Does such an
option exist?

https://codereview.appspot.com/561340043/

Reply | Threaded
Open this post in threaded view
|

Re: Add a tentative .clang-format for LilyPond. (issue 561340043 by hanwenn@gmail.com)

nine.fierce.ballads
In reply to this post by Dev mailing list
On 2020/01/29 07:41:40, lemzwerg wrote:
> The output looks good.  BTW, for the sake of Emacs, it would be nice
if two
> spaces after a full dot could be retained in comments.  Does such an
option
> exist?

Retaining everything with ReflowComments:false might be the only way.
https://releases.llvm.org/9.0.0/tools/clang/docs/ClangFormatStyleOptions.html


https://codereview.appspot.com/561340043/

Reply | Threaded
Open this post in threaded view
|

Re: Add a tentative .clang-format for LilyPond. (issue 561340043 by hanwenn@gmail.com)

Han-Wen Nienhuys-3
In reply to this post by Dev mailing list
On 2020/01/29 12:19:54, Dan Eble wrote:
> On 2020/01/29 07:41:40, lemzwerg wrote:
> > The output looks good.  BTW, for the sake of Emacs, it would be nice
if two
> > spaces after a full dot could be retained in comments.  Does such an
option
> > exist?
>
> Retaining everything with ReflowComments:false might be the only way.
>
https://releases.llvm.org/9.0.0/tools/clang/docs/ClangFormatStyleOptions.html

I've applied your suggestion; PTAL.

https://codereview.appspot.com/561340043/

Reply | Threaded
Open this post in threaded view
|

Re: Add a tentative .clang-format for LilyPond. (issue 561340043 by hanwenn@gmail.com)

nine.fierce.ballads
In reply to this post by Dev mailing list
On 2020/01/31 09:39:33, hanwenn wrote:
> I've applied your suggestion; PTAL.

If Werner likes it, I'm fine with it.  I haven't tried it myself because
I want to avoid being drawn into discussing the details of the style,
but I like seeing movement toward a better process.


https://codereview.appspot.com/561340043/

Reply | Threaded
Open this post in threaded view
|

Re: Add a tentative .clang-format for LilyPond. (issue 561340043 by hanwenn@gmail.com)

Han-Wen Nienhuys-3
In reply to this post by Dev mailing list
On 2020/01/31 12:35:45, Dan Eble wrote:
> On 2020/01/31 09:39:33, hanwenn wrote:
> > I've applied your suggestion; PTAL.
>
> If Werner likes it, I'm fine with it.  I haven't tried it myself
because I want
> to avoid being drawn into discussing the details of the style, but I
like seeing
> movement toward a better process.

that's great to hear. Will james pick this up automatically now, or does
it need an LGTM?

https://codereview.appspot.com/561340043/

Reply | Threaded
Open this post in threaded view
|

Re: Add a tentative .clang-format for LilyPond. (issue 561340043 by hanwenn@gmail.com)

Carl Sorensen
In reply to this post by Dev mailing list
IIUC, this is a .clang-format that can be (but is not required to be)
used to format source code and prevent comments about formatting.

At this point, we are not enforcing a shift to clang-format as the code
standard for LilyPond.

If this is true, LGTM.

If we are enforcing a shift to clang-format, then I think we need an
LGTM from David K.

https://codereview.appspot.com/561340043/

Reply | Threaded
Open this post in threaded view
|

Re: Add a tentative .clang-format for LilyPond. (issue 561340043 by hanwenn@gmail.com)

Han-Wen Nienhuys-3
In reply to this post by Dev mailing list
On 2020/01/31 18:33:38, Carl wrote:
> IIUC, this is a .clang-format that can be (but is not required to be)
used to
> format source code and prevent comments about formatting.
>
> At this point, we are not enforcing a shift to clang-format as the
code standard
> for LilyPond.
>
> If this is true, LGTM.
>
> If we are enforcing a shift to clang-format, then I think we need an
LGTM from
> David K.

correct.

https://codereview.appspot.com/561340043/

Reply | Threaded
Open this post in threaded view
|

Re: Add a tentative .clang-format for LilyPond. (issue 561340043 by hanwenn@gmail.com)

Carl Sorensen
In reply to this post by Dev mailing list
On 2020/01/31 18:06:00, hanwenn wrote:

>  Will james pick this up automatically now, or does it need
> an LGTM?

James should pick it up automatically now.

https://codereview.appspot.com/561340043/

Reply | Threaded
Open this post in threaded view
|

Re: Add a tentative .clang-format for LilyPond. (issue 561340043 by hanwenn@gmail.com)

Dev mailing list
In reply to this post by Dev mailing list
> If Werner likes it, I'm fine with it.

I do like it, and it is completely non-intrusive since it gets used
locally only for those people who set up a proper git hook (or call
`clang-format` manually).

https://codereview.appspot.com/561340043/

Reply | Threaded
Open this post in threaded view
|

Re: Add a tentative .clang-format for LilyPond. (issue 561340043 by hanwenn@gmail.com)

nine.fierce.ballads
In reply to this post by Dev mailing list
I'm running some of my patches through clang-format as I prepare to push
them.

This is an example of a kind of change it wants to make:

-  const array<int, 2> key {column_rank, dir};
+  const array<int, 2> key{column_rank, dir};

Note the space after key.  The setting that controls this is
SpaceBeforeCpp11BracedList.  Am I correct that we should use a space
here for consistency with historical style?

More examples of the effect of the option are in the docs:
https://releases.llvm.org/9.0.0/tools/clang/docs/ClangFormatStyleOptions.html

https://codereview.appspot.com/561340043/

Reply | Threaded
Open this post in threaded view
|

Re: Add a tentative .clang-format for LilyPond. (issue 561340043 by hanwenn@gmail.com)

Han-Wen Nienhuys-3
In reply to this post by Dev mailing list
On 2020/02/04 20:35:40, Dan Eble wrote:
> I'm running some of my patches through clang-format as I prepare to
push them.
>
> This is an example of a kind of change it wants to make:
>
> -  const array<int, 2> key {column_rank, dir};
> +  const array<int, 2> key{column_rank, dir};
>
> Note the space after key.  The setting that controls this is
> SpaceBeforeCpp11BracedList.  Am I correct that we should use a space
here for
> consistency with historical style?
>
> More examples of the effect of the option are in the docs:
>
https://releases.llvm.org/9.0.0/tools/clang/docs/ClangFormatStyleOptions.html

historically, we didn't have C++11, so this wasn't an issue. I'm fine
with whatever you pick.

With the space seems more coherent with the s p a c e y  GNU style.

https://codereview.appspot.com/561340043/

Reply | Threaded
Open this post in threaded view
|

Re: Add a tentative .clang-format for LilyPond. (issue 561340043 by hanwenn@gmail.com)

David Kastrup
In reply to this post by Dev mailing list
On 2020/02/04 22:18:23, hanwenn wrote:
> On 2020/02/04 20:35:40, Dan Eble wrote:
> > I'm running some of my patches through clang-format as I prepare to
push them.
> >
> > This is an example of a kind of change it wants to make:
> >
> > -  const array<int, 2> key {column_rank, dir};
> > +  const array<int, 2> key{column_rank, dir};
> >
> > Note the space after key.  The setting that controls this is
> > SpaceBeforeCpp11BracedList.  Am I correct that we should use a space
here for
> > consistency with historical style?
> >
> > More examples of the effect of the option are in the docs:
> >
https://releases.llvm.org/9.0.0/tools/clang/docs/ClangFormatStyleOptions.html
>
> historically, we didn't have C++11, so this wasn't an issue. I'm fine
with
> whatever you pick.
>
> With the space seems more coherent with the s p a c e y  GNU style.

Personally, the best of two worlds would be if a ping-pong between our
current Astyle and clang-format would end up stable after one
application of each.  That way, putting the code base through both
Astyle and clang-format semi-regularly would put it into a form where
users of either formatting tool could contribute their changes without
affecting already formatted code in the same file.

https://codereview.appspot.com/561340043/

Reply | Threaded
Open this post in threaded view
|

Re: Add a tentative .clang-format for LilyPond. (issue 561340043 by hanwenn@gmail.com)

nine.fierce.ballads
In reply to this post by Dev mailing list
On 2020/02/04 23:40:03, dak wrote:
> Personally, the best of two worlds would be if a ping-pong between our
current
> Astyle and clang-format would end up stable after one application of
each.  That
> way, putting the code base through both Astyle and clang-format
semi-regularly
> would put it into a form where users of either formatting tool could
contribute
> their changes without affecting already formatted code in the same
file.

+1.  I actually tried that earlier today, and there were lots of
differences.  I was using astyle 3.1 because that's what I had, so I'm
not sure it represents the ideal, but I'm confident that this
clang-format file needs some work regardless.  It's a good start,
though.

https://codereview.appspot.com/561340043/