[Qemu-devel] [RFC PATCH] Add qemu .clang-format

[Qemu-devel] [RFC PATCH] Add qemu .clang-format

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

clang-format is awesome to reflow your code according to qemu coding
style in an editor (in the region you modify).

(note: clang-tidy should be able to add missing braces around
statements, but I haven't tried it, it's quite recent)

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 .clang-format | 6 ++++++
 1 file changed, 6 insertions(+)
 create mode 100644 .clang-format

diff --git a/.clang-format b/.clang-format
new file mode 100644
index 0000000..6422547
--- /dev/null
+++ b/.clang-format
@@ -0,0 +1,6 @@
+BasedOnStyle: LLVM
+IndentWidth: 4
+UseTab: Never
+BreakBeforeBraces: Linux
+AllowShortIfStatementsOnASingleLine: false
+IndentCaseLabels: false
-- 
2.4.3

Re: [Qemu-devel] [RFC PATCH] Add qemu .clang-format

[Wang, Lei]

On 10/2/2015 1:30 AM, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> clang-format is awesome to reflow your code according to qemu coding
> style in an editor (in the region you modify).
> 
> (note: clang-tidy should be able to add missing braces around
> statements, but I haven't tried it, it's quite recent)
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>   .clang-format | 6 ++++++
>   1 file changed, 6 insertions(+)
>   create mode 100644 .clang-format
> 
> diff --git a/.clang-format b/.clang-format
> new file mode 100644
> index 0000000..6422547
> --- /dev/null
> +++ b/.clang-format
> @@ -0,0 +1,6 @@
> +BasedOnStyle: LLVM
> +IndentWidth: 4
> +UseTab: Never
> +BreakBeforeBraces: Linux
> +AllowShortIfStatementsOnASingleLine: false
> +IndentCaseLabels: false

Hi, any progress on this? I also found a gist on GitHub which can be a 
reference: https://gist.github.com/elmarco/aa5e0b23567f46fb7f0e73cde586a0c1

Re: [Qemu-devel] [RFC PATCH] Add qemu .clang-format

[Daniel P. Berrangé]

On Wed, Aug 31, 2022 at 02:23:51PM +0800, Wang, Lei wrote:
> 
> On 10/2/2015 1:30 AM, marcandre.lureau@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> > 
> > clang-format is awesome to reflow your code according to qemu coding
> > style in an editor (in the region you modify).
> > 
> > (note: clang-tidy should be able to add missing braces around
> > statements, but I haven't tried it, it's quite recent)
> > 
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >   .clang-format | 6 ++++++
> >   1 file changed, 6 insertions(+)
> >   create mode 100644 .clang-format
> > 
> > diff --git a/.clang-format b/.clang-format
> > new file mode 100644
> > index 0000000..6422547
> > --- /dev/null
> > +++ b/.clang-format
> > @@ -0,0 +1,6 @@
> > +BasedOnStyle: LLVM
> > +IndentWidth: 4
> > +UseTab: Never
> > +BreakBeforeBraces: Linux
> > +AllowShortIfStatementsOnASingleLine: false
> > +IndentCaseLabels: false
> 
> Hi, any progress on this? I also found a gist on GitHub which can be a
> reference: https://gist.github.com/elmarco/aa5e0b23567f46fb7f0e73cde586a0c1

clang-format is a great tool and I'd highly recommend its use on
any newly started projects, and even retrospectively on existing
projects which are small scale. Adding it to large existing projects
is problematic though.

None of the QEMU code complies with it today and indeed there is
quite a bit of style variance across different parts of QEMU. If
we add this config file, and someone makes a 1 line change in a
file, clang-format will reformat the entire file contents.

The only practical way to introduce use of clang-format would be
to do a bulk reformat of the entire codebase. That is something
that is quite disruptive to both people with patches they're
working on but not submitted yet, as well as people wanting to
cherry-pick new commits back to old code branches.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [RFC PATCH] Add qemu .clang-format

[Wang, Lei]

On 8/31/2022 4:49 PM, Daniel P. Berrangé wrote:
> On Wed, Aug 31, 2022 at 02:23:51PM +0800, Wang, Lei wrote:
>>
>> On 10/2/2015 1:30 AM, marcandre.lureau@redhat.com wrote:
>>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>
>>> clang-format is awesome to reflow your code according to qemu coding
>>> style in an editor (in the region you modify).
>>>
>>> (note: clang-tidy should be able to add missing braces around
>>> statements, but I haven't tried it, it's quite recent)
>>>
>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>> ---
>>>    .clang-format | 6 ++++++
>>>    1 file changed, 6 insertions(+)
>>>    create mode 100644 .clang-format
>>>
>>> diff --git a/.clang-format b/.clang-format
>>> new file mode 100644
>>> index 0000000..6422547
>>> --- /dev/null
>>> +++ b/.clang-format
>>> @@ -0,0 +1,6 @@
>>> +BasedOnStyle: LLVM
>>> +IndentWidth: 4
>>> +UseTab: Never
>>> +BreakBeforeBraces: Linux
>>> +AllowShortIfStatementsOnASingleLine: false
>>> +IndentCaseLabels: false
>>
>> Hi, any progress on this? I also found a gist on GitHub which can be a
>> reference: https://gist.github.com/elmarco/aa5e0b23567f46fb7f0e73cde586a0c1
> 
> clang-format is a great tool and I'd highly recommend its use on
> any newly started projects, and even retrospectively on existing
> projects which are small scale. Adding it to large existing projects
> is problematic though.
> 
> None of the QEMU code complies with it today and indeed there is
> quite a bit of style variance across different parts of QEMU. If
> we add this config file, and someone makes a 1 line change in a
> file, clang-format will reformat the entire file contents.
> 
> The only practical way to introduce use of clang-format would be
> to do a bulk reformat of the entire codebase. That is something
> that is quite disruptive to both people with patches they're
> working on but not submitted yet, as well as people wanting to
> cherry-pick new commits back to old code branches.
> 
> With regards,
> Daniel

I think the benefits of introducing clang-format mainly for its ability 
to format a code range, which means for any future contributions, we 
could encourage a range format before the patch is generated. This can 
extensively simplify my workflow, especially because I use the Neovim + 
LSP combination, which supports a built-in function "lua 
vim.lsp.buf.range_formatting()".

I have no interest in reformatting the existing code and also think 
using it to reformat an entire file shouldn't be encouraged, but, we can 
leverage this tool to give future contributions a better experience. 
It's also important to note that the kernel already has a 
".clang-format" file, so I think we can give it a try:)

BR,
Lei

Re: [Qemu-devel] [RFC PATCH] Add qemu .clang-format

[Daniel P. Berrangé]

On Wed, Aug 31, 2022 at 05:18:34PM +0800, Wang, Lei wrote:
> 
> 
> On 8/31/2022 4:49 PM, Daniel P. Berrangé wrote:
> > On Wed, Aug 31, 2022 at 02:23:51PM +0800, Wang, Lei wrote:
> > > 
> > > On 10/2/2015 1:30 AM, marcandre.lureau@redhat.com wrote:
> > > > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > > 
> > > > clang-format is awesome to reflow your code according to qemu coding
> > > > style in an editor (in the region you modify).
> > > > 
> > > > (note: clang-tidy should be able to add missing braces around
> > > > statements, but I haven't tried it, it's quite recent)
> > > > 
> > > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > > ---
> > > >    .clang-format | 6 ++++++
> > > >    1 file changed, 6 insertions(+)
> > > >    create mode 100644 .clang-format
> > > > 
> > > > diff --git a/.clang-format b/.clang-format
> > > > new file mode 100644
> > > > index 0000000..6422547
> > > > --- /dev/null
> > > > +++ b/.clang-format
> > > > @@ -0,0 +1,6 @@
> > > > +BasedOnStyle: LLVM
> > > > +IndentWidth: 4
> > > > +UseTab: Never
> > > > +BreakBeforeBraces: Linux
> > > > +AllowShortIfStatementsOnASingleLine: false
> > > > +IndentCaseLabels: false
> > > 
> > > Hi, any progress on this? I also found a gist on GitHub which can be a
> > > reference: https://gist.github.com/elmarco/aa5e0b23567f46fb7f0e73cde586a0c1
> > 
> > clang-format is a great tool and I'd highly recommend its use on
> > any newly started projects, and even retrospectively on existing
> > projects which are small scale. Adding it to large existing projects
> > is problematic though.
> > 
> > None of the QEMU code complies with it today and indeed there is
> > quite a bit of style variance across different parts of QEMU. If
> > we add this config file, and someone makes a 1 line change in a
> > file, clang-format will reformat the entire file contents.
> > 
> > The only practical way to introduce use of clang-format would be
> > to do a bulk reformat of the entire codebase. That is something
> > that is quite disruptive to both people with patches they're
> > working on but not submitted yet, as well as people wanting to
> > cherry-pick new commits back to old code branches.
> > 
> > With regards,
> > Daniel
> 
> I think the benefits of introducing clang-format mainly for its ability to
> format a code range, which means for any future contributions, we could
> encourage a range format before the patch is generated. This can extensively
> simplify my workflow, especially because I use the Neovim + LSP combination,
> which supports a built-in function "lua vim.lsp.buf.range_formatting()".

IMHO partial format conversions are even worse than full conversions,
because they would make code inconsistent within the scope of a file.

> I have no interest in reformatting the existing code and also think using it
> to reformat an entire file shouldn't be encouraged, but, we can leverage
> this tool to give future contributions a better experience. It's also
> important to note that the kernel already has a ".clang-format" file, so I
> think we can give it a try:)

The mere action of introducing a .clang-format file in the root of the
repository will cause some contributors' editors to automatically
reformat files every time they are saved. IOW even if you don't want
intend to do reformatting, that will be a net result.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [RFC PATCH] Add qemu .clang-format

[Wang, Lei]

On 8/31/2022 6:39 PM, Daniel P. Berrangé wrote:
> On Wed, Aug 31, 2022 at 05:18:34PM +0800, Wang, Lei wrote:
>>
>>
>> On 8/31/2022 4:49 PM, Daniel P. Berrangé wrote:
>>> On Wed, Aug 31, 2022 at 02:23:51PM +0800, Wang, Lei wrote:
>>>>
>>>> On 10/2/2015 1:30 AM, marcandre.lureau@redhat.com wrote:
>>>>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>>>
>>>>> clang-format is awesome to reflow your code according to qemu coding
>>>>> style in an editor (in the region you modify).
>>>>>
>>>>> (note: clang-tidy should be able to add missing braces around
>>>>> statements, but I haven't tried it, it's quite recent)
>>>>>
>>>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>>> ---
>>>>>     .clang-format | 6 ++++++
>>>>>     1 file changed, 6 insertions(+)
>>>>>     create mode 100644 .clang-format
>>>>>
>>>>> diff --git a/.clang-format b/.clang-format
>>>>> new file mode 100644
>>>>> index 0000000..6422547
>>>>> --- /dev/null
>>>>> +++ b/.clang-format
>>>>> @@ -0,0 +1,6 @@
>>>>> +BasedOnStyle: LLVM
>>>>> +IndentWidth: 4
>>>>> +UseTab: Never
>>>>> +BreakBeforeBraces: Linux
>>>>> +AllowShortIfStatementsOnASingleLine: false
>>>>> +IndentCaseLabels: false
>>>>
>>>> Hi, any progress on this? I also found a gist on GitHub which can be a
>>>> reference: https://gist.github.com/elmarco/aa5e0b23567f46fb7f0e73cde586a0c1
>>>
>>> clang-format is a great tool and I'd highly recommend its use on
>>> any newly started projects, and even retrospectively on existing
>>> projects which are small scale. Adding it to large existing projects
>>> is problematic though.
>>>
>>> None of the QEMU code complies with it today and indeed there is
>>> quite a bit of style variance across different parts of QEMU. If
>>> we add this config file, and someone makes a 1 line change in a
>>> file, clang-format will reformat the entire file contents.
>>>
>>> The only practical way to introduce use of clang-format would be
>>> to do a bulk reformat of the entire codebase. That is something
>>> that is quite disruptive to both people with patches they're
>>> working on but not submitted yet, as well as people wanting to
>>> cherry-pick new commits back to old code branches.
>>>
>>> With regards,
>>> Daniel
>>
>> I think the benefits of introducing clang-format mainly for its ability to
>> format a code range, which means for any future contributions, we could
>> encourage a range format before the patch is generated. This can extensively
>> simplify my workflow, especially because I use the Neovim + LSP combination,
>> which supports a built-in function "lua vim.lsp.buf.range_formatting()".
> 
> IMHO partial format conversions are even worse than full conversions,
> because they would make code inconsistent within the scope of a file.

So you mean when we're adding new code in an old file, the coding style 
should also be the old one? That sounds a bit unreasonable. I thought we 
are shifting the coding style in an on-demand way, so we can finally 
achieve to the new style mildly, if each time we're using the old coding 
style, that could be impossible.

>> I have no interest in reformatting the existing code and also think using it
>> to reformat an entire file shouldn't be encouraged, but, we can leverage
>> this tool to give future contributions a better experience. It's also
>> important to note that the kernel already has a ".clang-format" file, so I
>> think we can give it a try:)
> 
> The mere action of introducing a .clang-format file in the root of the
> repository will cause some contributors' editors to automatically
> reformat files every time they are saved. IOW even if you don't want
> intend to do reformatting, that will be a net result.
> 
> With regards,
> Daniel

I think that depends on developer's configuration, as far as I know, 
format on save is a feature which can be easily disabled on most of the 
IDE's, such as VSCode.

Re: [Qemu-devel] [RFC PATCH] Add qemu .clang-format

[Daniel P. Berrangé]

On Thu, Sep 01, 2022 at 09:08:33AM +0800, Wang, Lei wrote:
> On 8/31/2022 6:39 PM, Daniel P. Berrangé wrote:
> > On Wed, Aug 31, 2022 at 05:18:34PM +0800, Wang, Lei wrote:
> > > 
> > > 
> > > On 8/31/2022 4:49 PM, Daniel P. Berrangé wrote:
> > > > On Wed, Aug 31, 2022 at 02:23:51PM +0800, Wang, Lei wrote:
> > > > > 
> > > > > On 10/2/2015 1:30 AM, marcandre.lureau@redhat.com wrote:
> > > > > > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > > > > 
> > > > > > clang-format is awesome to reflow your code according to qemu coding
> > > > > > style in an editor (in the region you modify).
> > > > > > 
> > > > > > (note: clang-tidy should be able to add missing braces around
> > > > > > statements, but I haven't tried it, it's quite recent)
> > > > > > 
> > > > > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > > > > ---
> > > > > >     .clang-format | 6 ++++++
> > > > > >     1 file changed, 6 insertions(+)
> > > > > >     create mode 100644 .clang-format
> > > > > > 
> > > > > > diff --git a/.clang-format b/.clang-format
> > > > > > new file mode 100644
> > > > > > index 0000000..6422547
> > > > > > --- /dev/null
> > > > > > +++ b/.clang-format
> > > > > > @@ -0,0 +1,6 @@
> > > > > > +BasedOnStyle: LLVM
> > > > > > +IndentWidth: 4
> > > > > > +UseTab: Never
> > > > > > +BreakBeforeBraces: Linux
> > > > > > +AllowShortIfStatementsOnASingleLine: false
> > > > > > +IndentCaseLabels: false
> > > > > 
> > > > > Hi, any progress on this? I also found a gist on GitHub which can be a
> > > > > reference: https://gist.github.com/elmarco/aa5e0b23567f46fb7f0e73cde586a0c1
> > > > 
> > > > clang-format is a great tool and I'd highly recommend its use on
> > > > any newly started projects, and even retrospectively on existing
> > > > projects which are small scale. Adding it to large existing projects
> > > > is problematic though.
> > > > 
> > > > None of the QEMU code complies with it today and indeed there is
> > > > quite a bit of style variance across different parts of QEMU. If
> > > > we add this config file, and someone makes a 1 line change in a
> > > > file, clang-format will reformat the entire file contents.
> > > > 
> > > > The only practical way to introduce use of clang-format would be
> > > > to do a bulk reformat of the entire codebase. That is something
> > > > that is quite disruptive to both people with patches they're
> > > > working on but not submitted yet, as well as people wanting to
> > > > cherry-pick new commits back to old code branches.
> > > > 
> > > > With regards,
> > > > Daniel
> > > 
> > > I think the benefits of introducing clang-format mainly for its ability to
> > > format a code range, which means for any future contributions, we could
> > > encourage a range format before the patch is generated. This can extensively
> > > simplify my workflow, especially because I use the Neovim + LSP combination,
> > > which supports a built-in function "lua vim.lsp.buf.range_formatting()".
> > 
> > IMHO partial format conversions are even worse than full conversions,
> > because they would make code inconsistent within the scope of a file.
> 
> So you mean when we're adding new code in an old file, the coding style
> should also be the old one? That sounds a bit unreasonable. I thought we are
> shifting the coding style in an on-demand way, so we can finally achieve to
> the new style mildly, if each time we're using the old coding style, that
> could be impossible.

From my POV as a maintainer, the best situation would be consistency across
the entire codebase. Since we likely won't get that though, then next best
is consistency across the subsystem directory, and next best is consistency
across the whole file.  Mixing code styles within a file is the worst IMHO.

> 
> > > I have no interest in reformatting the existing code and also think using it
> > > to reformat an entire file shouldn't be encouraged, but, we can leverage
> > > this tool to give future contributions a better experience. It's also
> > > important to note that the kernel already has a ".clang-format" file, so I
> > > think we can give it a try:)
> > 
> > The mere action of introducing a .clang-format file in the root of the
> > repository will cause some contributors' editors to automatically
> > reformat files every time they are saved. IOW even if you don't want
> > intend to do reformatting, that will be a net result.
> 
> I think that depends on developer's configuration, as far as I know, format
> on save is a feature which can be easily disabled on most of the IDE's, such
> as VSCode.

You could disable it, but it requires each developer to know that we're
shipping a clang-format that should not in fact be used to reformat
code, which is rather counterintuitive. 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [RFC PATCH] Add qemu .clang-format

[Wang, Lei]

On 9/1/2022 4:12 PM, Daniel P. Berrangé wrote:
> On Thu, Sep 01, 2022 at 09:08:33AM +0800, Wang, Lei wrote:
>> On 8/31/2022 6:39 PM, Daniel P. Berrangé wrote:
>>> On Wed, Aug 31, 2022 at 05:18:34PM +0800, Wang, Lei wrote:
>>>>
>>>>
>>>> On 8/31/2022 4:49 PM, Daniel P. Berrangé wrote:
>>>>> On Wed, Aug 31, 2022 at 02:23:51PM +0800, Wang, Lei wrote:
>>>>>>
>>>>>> On 10/2/2015 1:30 AM, marcandre.lureau@redhat.com wrote:
>>>>>>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>>>>>
>>>>>>> clang-format is awesome to reflow your code according to qemu coding
>>>>>>> style in an editor (in the region you modify).
>>>>>>>
>>>>>>> (note: clang-tidy should be able to add missing braces around
>>>>>>> statements, but I haven't tried it, it's quite recent)
>>>>>>>
>>>>>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>>>>> ---
>>>>>>>     .clang-format | 6 ++++++
>>>>>>>     1 file changed, 6 insertions(+)
>>>>>>>     create mode 100644 .clang-format
>>>>>>>
>>>>>>> diff --git a/.clang-format b/.clang-format
>>>>>>> new file mode 100644
>>>>>>> index 0000000..6422547
>>>>>>> --- /dev/null
>>>>>>> +++ b/.clang-format
>>>>>>> @@ -0,0 +1,6 @@
>>>>>>> +BasedOnStyle: LLVM
>>>>>>> +IndentWidth: 4
>>>>>>> +UseTab: Never
>>>>>>> +BreakBeforeBraces: Linux
>>>>>>> +AllowShortIfStatementsOnASingleLine: false
>>>>>>> +IndentCaseLabels: false
>>>>>>
>>>>>> Hi, any progress on this? I also found a gist on GitHub which can be a
>>>>>> reference: https://gist.github.com/elmarco/aa5e0b23567f46fb7f0e73cde586a0c1
>>>>>
>>>>> clang-format is a great tool and I'd highly recommend its use on
>>>>> any newly started projects, and even retrospectively on existing
>>>>> projects which are small scale. Adding it to large existing projects
>>>>> is problematic though.
>>>>>
>>>>> None of the QEMU code complies with it today and indeed there is
>>>>> quite a bit of style variance across different parts of QEMU. If
>>>>> we add this config file, and someone makes a 1 line change in a
>>>>> file, clang-format will reformat the entire file contents.
>>>>>
>>>>> The only practical way to introduce use of clang-format would be
>>>>> to do a bulk reformat of the entire codebase. That is something
>>>>> that is quite disruptive to both people with patches they're
>>>>> working on but not submitted yet, as well as people wanting to
>>>>> cherry-pick new commits back to old code branches.
>>>>>
>>>>> With regards,
>>>>> Daniel
>>>>
>>>> I think the benefits of introducing clang-format mainly for its ability to
>>>> format a code range, which means for any future contributions, we could
>>>> encourage a range format before the patch is generated. This can extensively
>>>> simplify my workflow, especially because I use the Neovim + LSP combination,
>>>> which supports a built-in function "lua vim.lsp.buf.range_formatting()".
>>>
>>> IMHO partial format conversions are even worse than full conversions,
>>> because they would make code inconsistent within the scope of a file.
>>
>> So you mean when we're adding new code in an old file, the coding style
>> should also be the old one? That sounds a bit unreasonable. I thought we are
>> shifting the coding style in an on-demand way, so we can finally achieve to
>> the new style mildly, if each time we're using the old coding style, that
>> could be impossible.
> 
> From my POV as a maintainer, the best situation would be consistency across
> the entire codebase. Since we likely won't get that though, then next best
> is consistency across the subsystem directory, and next best is consistency
> across the whole file.  Mixing code styles within a file is the worst IMHO.
> 
>>
>>>> I have no interest in reformatting the existing code and also think using it
>>>> to reformat an entire file shouldn't be encouraged, but, we can leverage
>>>> this tool to give future contributions a better experience. It's also
>>>> important to note that the kernel already has a ".clang-format" file, so I
>>>> think we can give it a try:)
>>>
>>> The mere action of introducing a .clang-format file in the root of the
>>> repository will cause some contributors' editors to automatically
>>> reformat files every time they are saved. IOW even if you don't want
>>> intend to do reformatting, that will be a net result.
>>
>> I think that depends on developer's configuration, as far as I know, format
>> on save is a feature which can be easily disabled on most of the IDE's, such
>> as VSCode.
> 
> You could disable it, but it requires each developer to know that we're
> shipping a clang-format that should not in fact be used to reformat
> code, which is rather counterintuitive. 
> 
> With regards,
> Daniel

OK, your POV makes sense too. I think we can do a tradeoff, for an example, we
can add an officially suggested ".clang-format" file in the documentation, so it
won't confuse the developers who have no interest in the clang stuffs, and it
will also be more convenient for the developers who don't want to check the
coding style manually each time before they're submitting a patch.

BR,
Lei

Re: [Qemu-devel] [RFC PATCH] Add qemu .clang-format

[Alex Bennée]

"Wang, Lei" <lei4.wang@intel.com> writes:

> On 9/1/2022 4:12 PM, Daniel P. Berrangé wrote:
>> On Thu, Sep 01, 2022 at 09:08:33AM +0800, Wang, Lei wrote:
>>> On 8/31/2022 6:39 PM, Daniel P. Berrangé wrote:
>>>> On Wed, Aug 31, 2022 at 05:18:34PM +0800, Wang, Lei wrote:
>>>>>
>>>>>
>>>>> On 8/31/2022 4:49 PM, Daniel P. Berrangé wrote:
>>>>>> On Wed, Aug 31, 2022 at 02:23:51PM +0800, Wang, Lei wrote:
>>>>>>>
>>>>>>> On 10/2/2015 1:30 AM, marcandre.lureau@redhat.com wrote:
>>>>>>>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>>>>>>
>>>>>>>> clang-format is awesome to reflow your code according to qemu coding
>>>>>>>> style in an editor (in the region you modify).
>>>>>>>>
>>>>>>>> (note: clang-tidy should be able to add missing braces around
>>>>>>>> statements, but I haven't tried it, it's quite recent)
>>>>>>>>
>>>>>>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>>>>>> ---
>>>>>>>>     .clang-format | 6 ++++++
>>>>>>>>     1 file changed, 6 insertions(+)
>>>>>>>>     create mode 100644 .clang-format
>>>>>>>>
>>>>>>>> diff --git a/.clang-format b/.clang-format
>>>>>>>> new file mode 100644
>>>>>>>> index 0000000..6422547
>>>>>>>> --- /dev/null
>>>>>>>> +++ b/.clang-format
>>>>>>>> @@ -0,0 +1,6 @@
>>>>>>>> +BasedOnStyle: LLVM
>>>>>>>> +IndentWidth: 4
>>>>>>>> +UseTab: Never
>>>>>>>> +BreakBeforeBraces: Linux
>>>>>>>> +AllowShortIfStatementsOnASingleLine: false
>>>>>>>> +IndentCaseLabels: false
>>>>>>>
>>>>>>> Hi, any progress on this? I also found a gist on GitHub which can be a
>>>>>>> reference: https://gist.github.com/elmarco/aa5e0b23567f46fb7f0e73cde586a0c1
>>>>>>
>>>>>> clang-format is a great tool and I'd highly recommend its use on
>>>>>> any newly started projects, and even retrospectively on existing
>>>>>> projects which are small scale. Adding it to large existing projects
>>>>>> is problematic though.
>>>>>>
>>>>>> None of the QEMU code complies with it today and indeed there is
>>>>>> quite a bit of style variance across different parts of QEMU. If
>>>>>> we add this config file, and someone makes a 1 line change in a
>>>>>> file, clang-format will reformat the entire file contents.
>>>>>>
>>>>>> The only practical way to introduce use of clang-format would be
>>>>>> to do a bulk reformat of the entire codebase. That is something
>>>>>> that is quite disruptive to both people with patches they're
>>>>>> working on but not submitted yet, as well as people wanting to
>>>>>> cherry-pick new commits back to old code branches.
>>>>>>
>>>>>> With regards,
>>>>>> Daniel
>>>>>
>>>>> I think the benefits of introducing clang-format mainly for its ability to
>>>>> format a code range, which means for any future contributions, we could
>>>>> encourage a range format before the patch is generated. This can extensively
>>>>> simplify my workflow, especially because I use the Neovim + LSP combination,
>>>>> which supports a built-in function "lua vim.lsp.buf.range_formatting()".
>>>>
>>>> IMHO partial format conversions are even worse than full conversions,
>>>> because they would make code inconsistent within the scope of a file.
>>>
>>> So you mean when we're adding new code in an old file, the coding style
>>> should also be the old one? That sounds a bit unreasonable. I thought we are
>>> shifting the coding style in an on-demand way, so we can finally achieve to
>>> the new style mildly, if each time we're using the old coding style, that
>>> could be impossible.
>> 
>> From my POV as a maintainer, the best situation would be consistency across
>> the entire codebase. Since we likely won't get that though, then next best
>> is consistency across the subsystem directory, and next best is consistency
>> across the whole file.  Mixing code styles within a file is the worst IMHO.
>> 
>>>
>>>>> I have no interest in reformatting the existing code and also think using it
>>>>> to reformat an entire file shouldn't be encouraged, but, we can leverage
>>>>> this tool to give future contributions a better experience. It's also
>>>>> important to note that the kernel already has a ".clang-format" file, so I
>>>>> think we can give it a try:)
>>>>
>>>> The mere action of introducing a .clang-format file in the root of the
>>>> repository will cause some contributors' editors to automatically
>>>> reformat files every time they are saved. IOW even if you don't want
>>>> intend to do reformatting, that will be a net result.
>>>
>>> I think that depends on developer's configuration, as far as I know, format
>>> on save is a feature which can be easily disabled on most of the IDE's, such
>>> as VSCode.
>> 
>> You could disable it, but it requires each developer to know that we're
>> shipping a clang-format that should not in fact be used to reformat
>> code, which is rather counterintuitive. 
>> 
>> With regards,
>> Daniel
>
> OK, your POV makes sense too. I think we can do a tradeoff, for an example, we
> can add an officially suggested ".clang-format" file in the documentation, so it
> won't confuse the developers who have no interest in the clang stuffs, and it
> will also be more convenient for the developers who don't want to check the
> coding style manually each time before they're submitting a patch.

For most editors we already have a .editorconfig but it looks like there
is no integration for clang-format there. We could put a file in
contrib/style/ for an explicit call:

  clang-format -style=contrib/style/clang.format

I suspect we should move the .dir-locals there to given Emacs users
should be able to use the .editorconfig and it reduces duplication.

And of course mention the location of these style linters in
docs/devel/style.rst

>
> BR,
> Lei


-- 
Alex Bennée

Re: [Qemu-devel] [RFC PATCH] Add qemu .clang-format

[Wang, Lei]

On 9/1/2022 7:55 PM, Alex Bennée wrote:
> 
> "Wang, Lei" <lei4.wang@intel.com> writes:
> 
>> On 9/1/2022 4:12 PM, Daniel P. Berrangé wrote:
>>> On Thu, Sep 01, 2022 at 09:08:33AM +0800, Wang, Lei wrote:
>>>> On 8/31/2022 6:39 PM, Daniel P. Berrangé wrote:
>>>>> On Wed, Aug 31, 2022 at 05:18:34PM +0800, Wang, Lei wrote:
>>>>>>
>>>>>>
>>>>>> On 8/31/2022 4:49 PM, Daniel P. Berrangé wrote:
>>>>>>> On Wed, Aug 31, 2022 at 02:23:51PM +0800, Wang, Lei wrote:
>>>>>>>>
>>>>>>>> On 10/2/2015 1:30 AM, marcandre.lureau@redhat.com wrote:
>>>>>>>>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>>>>>>>
>>>>>>>>> clang-format is awesome to reflow your code according to qemu coding
>>>>>>>>> style in an editor (in the region you modify).
>>>>>>>>>
>>>>>>>>> (note: clang-tidy should be able to add missing braces around
>>>>>>>>> statements, but I haven't tried it, it's quite recent)
>>>>>>>>>
>>>>>>>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>>>>>>> ---
>>>>>>>>>     .clang-format | 6 ++++++
>>>>>>>>>     1 file changed, 6 insertions(+)
>>>>>>>>>     create mode 100644 .clang-format
>>>>>>>>>
>>>>>>>>> diff --git a/.clang-format b/.clang-format
>>>>>>>>> new file mode 100644
>>>>>>>>> index 0000000..6422547
>>>>>>>>> --- /dev/null
>>>>>>>>> +++ b/.clang-format
>>>>>>>>> @@ -0,0 +1,6 @@
>>>>>>>>> +BasedOnStyle: LLVM
>>>>>>>>> +IndentWidth: 4
>>>>>>>>> +UseTab: Never
>>>>>>>>> +BreakBeforeBraces: Linux
>>>>>>>>> +AllowShortIfStatementsOnASingleLine: false
>>>>>>>>> +IndentCaseLabels: false
>>>>>>>>
>>>>>>>> Hi, any progress on this? I also found a gist on GitHub which can be a
>>>>>>>> reference: https://gist.github.com/elmarco/aa5e0b23567f46fb7f0e73cde586a0c1
>>>>>>>
>>>>>>> clang-format is a great tool and I'd highly recommend its use on
>>>>>>> any newly started projects, and even retrospectively on existing
>>>>>>> projects which are small scale. Adding it to large existing projects
>>>>>>> is problematic though.
>>>>>>>
>>>>>>> None of the QEMU code complies with it today and indeed there is
>>>>>>> quite a bit of style variance across different parts of QEMU. If
>>>>>>> we add this config file, and someone makes a 1 line change in a
>>>>>>> file, clang-format will reformat the entire file contents.
>>>>>>>
>>>>>>> The only practical way to introduce use of clang-format would be
>>>>>>> to do a bulk reformat of the entire codebase. That is something
>>>>>>> that is quite disruptive to both people with patches they're
>>>>>>> working on but not submitted yet, as well as people wanting to
>>>>>>> cherry-pick new commits back to old code branches.
>>>>>>>
>>>>>>> With regards,
>>>>>>> Daniel
>>>>>>
>>>>>> I think the benefits of introducing clang-format mainly for its ability to
>>>>>> format a code range, which means for any future contributions, we could
>>>>>> encourage a range format before the patch is generated. This can extensively
>>>>>> simplify my workflow, especially because I use the Neovim + LSP combination,
>>>>>> which supports a built-in function "lua vim.lsp.buf.range_formatting()".
>>>>>
>>>>> IMHO partial format conversions are even worse than full conversions,
>>>>> because they would make code inconsistent within the scope of a file.
>>>>
>>>> So you mean when we're adding new code in an old file, the coding style
>>>> should also be the old one? That sounds a bit unreasonable. I thought we are
>>>> shifting the coding style in an on-demand way, so we can finally achieve to
>>>> the new style mildly, if each time we're using the old coding style, that
>>>> could be impossible.
>>>
>>> From my POV as a maintainer, the best situation would be consistency across
>>> the entire codebase. Since we likely won't get that though, then next best
>>> is consistency across the subsystem directory, and next best is consistency
>>> across the whole file.  Mixing code styles within a file is the worst IMHO.
>>>
>>>>
>>>>>> I have no interest in reformatting the existing code and also think using it
>>>>>> to reformat an entire file shouldn't be encouraged, but, we can leverage
>>>>>> this tool to give future contributions a better experience. It's also
>>>>>> important to note that the kernel already has a ".clang-format" file, so I
>>>>>> think we can give it a try:)
>>>>>
>>>>> The mere action of introducing a .clang-format file in the root of the
>>>>> repository will cause some contributors' editors to automatically
>>>>> reformat files every time they are saved. IOW even if you don't want
>>>>> intend to do reformatting, that will be a net result.
>>>>
>>>> I think that depends on developer's configuration, as far as I know, format
>>>> on save is a feature which can be easily disabled on most of the IDE's, such
>>>> as VSCode.
>>>
>>> You could disable it, but it requires each developer to know that we're
>>> shipping a clang-format that should not in fact be used to reformat
>>> code, which is rather counterintuitive. 
>>>
>>> With regards,
>>> Daniel
>>
>> OK, your POV makes sense too. I think we can do a tradeoff, for an example, we
>> can add an officially suggested ".clang-format" file in the documentation, so it
>> won't confuse the developers who have no interest in the clang stuffs, and it
>> will also be more convenient for the developers who don't want to check the
>> coding style manually each time before they're submitting a patch.
> 
> For most editors we already have a .editorconfig but it looks like there
> is no integration for clang-format there. We could put a file in
> contrib/style/ for an explicit call:
> 
>   clang-format -style=contrib/style/clang.format
> 
> I suspect we should move the .dir-locals there to given Emacs users
> should be able to use the .editorconfig and it reduces duplication.

I'm not an Emacs guy, but it looks good to me.

> And of course mention the location of these style linters in
> docs/devel/style.rst

That's necessary indeed.

>>
>> BR,
>> Lei
> 
> 

Re: [Qemu-devel] [RFC PATCH] Add qemu .clang-format

[Daniel P. Berrangé]

On Thu, Sep 01, 2022 at 12:55:36PM +0100, Alex Bennée wrote:
> 
> "Wang, Lei" <lei4.wang@intel.com> writes:
> 
> > On 9/1/2022 4:12 PM, Daniel P. Berrangé wrote:
> >> On Thu, Sep 01, 2022 at 09:08:33AM +0800, Wang, Lei wrote:
> >>> On 8/31/2022 6:39 PM, Daniel P. Berrangé wrote:
> >>>> On Wed, Aug 31, 2022 at 05:18:34PM +0800, Wang, Lei wrote:
> >>>>>
> >>>>>
> >>>>> On 8/31/2022 4:49 PM, Daniel P. Berrangé wrote:
> >>>>>> On Wed, Aug 31, 2022 at 02:23:51PM +0800, Wang, Lei wrote:
> >>>>>>>
> >>>>>>> On 10/2/2015 1:30 AM, marcandre.lureau@redhat.com wrote:
> >>>>>>>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >>>>>>>>
> >>>>>>>> clang-format is awesome to reflow your code according to qemu coding
> >>>>>>>> style in an editor (in the region you modify).
> >>>>>>>>
> >>>>>>>> (note: clang-tidy should be able to add missing braces around
> >>>>>>>> statements, but I haven't tried it, it's quite recent)
> >>>>>>>>
> >>>>>>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> >>>>>>>> ---
> >>>>>>>>     .clang-format | 6 ++++++
> >>>>>>>>     1 file changed, 6 insertions(+)
> >>>>>>>>     create mode 100644 .clang-format
> >>>>>>>>
> >>>>>>>> diff --git a/.clang-format b/.clang-format
> >>>>>>>> new file mode 100644
> >>>>>>>> index 0000000..6422547
> >>>>>>>> --- /dev/null
> >>>>>>>> +++ b/.clang-format
> >>>>>>>> @@ -0,0 +1,6 @@
> >>>>>>>> +BasedOnStyle: LLVM
> >>>>>>>> +IndentWidth: 4
> >>>>>>>> +UseTab: Never
> >>>>>>>> +BreakBeforeBraces: Linux
> >>>>>>>> +AllowShortIfStatementsOnASingleLine: false
> >>>>>>>> +IndentCaseLabels: false
> >>>>>>>
> >>>>>>> Hi, any progress on this? I also found a gist on GitHub which can be a
> >>>>>>> reference: https://gist.github.com/elmarco/aa5e0b23567f46fb7f0e73cde586a0c1
> >>>>>>
> >>>>>> clang-format is a great tool and I'd highly recommend its use on
> >>>>>> any newly started projects, and even retrospectively on existing
> >>>>>> projects which are small scale. Adding it to large existing projects
> >>>>>> is problematic though.
> >>>>>>
> >>>>>> None of the QEMU code complies with it today and indeed there is
> >>>>>> quite a bit of style variance across different parts of QEMU. If
> >>>>>> we add this config file, and someone makes a 1 line change in a
> >>>>>> file, clang-format will reformat the entire file contents.
> >>>>>>
> >>>>>> The only practical way to introduce use of clang-format would be
> >>>>>> to do a bulk reformat of the entire codebase. That is something
> >>>>>> that is quite disruptive to both people with patches they're
> >>>>>> working on but not submitted yet, as well as people wanting to
> >>>>>> cherry-pick new commits back to old code branches.
> >>>>>>
> >>>>>> With regards,
> >>>>>> Daniel
> >>>>>
> >>>>> I think the benefits of introducing clang-format mainly for its ability to
> >>>>> format a code range, which means for any future contributions, we could
> >>>>> encourage a range format before the patch is generated. This can extensively
> >>>>> simplify my workflow, especially because I use the Neovim + LSP combination,
> >>>>> which supports a built-in function "lua vim.lsp.buf.range_formatting()".
> >>>>
> >>>> IMHO partial format conversions are even worse than full conversions,
> >>>> because they would make code inconsistent within the scope of a file.
> >>>
> >>> So you mean when we're adding new code in an old file, the coding style
> >>> should also be the old one? That sounds a bit unreasonable. I thought we are
> >>> shifting the coding style in an on-demand way, so we can finally achieve to
> >>> the new style mildly, if each time we're using the old coding style, that
> >>> could be impossible.
> >> 
> >> From my POV as a maintainer, the best situation would be consistency across
> >> the entire codebase. Since we likely won't get that though, then next best
> >> is consistency across the subsystem directory, and next best is consistency
> >> across the whole file.  Mixing code styles within a file is the worst IMHO.
> >> 
> >>>
> >>>>> I have no interest in reformatting the existing code and also think using it
> >>>>> to reformat an entire file shouldn't be encouraged, but, we can leverage
> >>>>> this tool to give future contributions a better experience. It's also
> >>>>> important to note that the kernel already has a ".clang-format" file, so I
> >>>>> think we can give it a try:)
> >>>>
> >>>> The mere action of introducing a .clang-format file in the root of the
> >>>> repository will cause some contributors' editors to automatically
> >>>> reformat files every time they are saved. IOW even if you don't want
> >>>> intend to do reformatting, that will be a net result.
> >>>
> >>> I think that depends on developer's configuration, as far as I know, format
> >>> on save is a feature which can be easily disabled on most of the IDE's, such
> >>> as VSCode.
> >> 
> >> You could disable it, but it requires each developer to know that we're
> >> shipping a clang-format that should not in fact be used to reformat
> >> code, which is rather counterintuitive. 
> >> 
> >> With regards,
> >> Daniel
> >
> > OK, your POV makes sense too. I think we can do a tradeoff, for an example, we
> > can add an officially suggested ".clang-format" file in the documentation, so it
> > won't confuse the developers who have no interest in the clang stuffs, and it
> > will also be more convenient for the developers who don't want to check the
> > coding style manually each time before they're submitting a patch.
> 
> For most editors we already have a .editorconfig but it looks like there
> is no integration for clang-format there. We could put a file in
> contrib/style/ for an explicit call:
> 
>   clang-format -style=contrib/style/clang.format
> 
> I suspect we should move the .dir-locals there to given Emacs users
> should be able to use the .editorconfig and it reduces duplication.

I'd be against that on the basis that EditorConfig support is an
add-on for emacs which we can't be sure our contributors will have
installed. The .dir-locals ensures we get sensible behaviour from
all emacs users by default.

> And of course mention the location of these style linters in
> docs/devel/style.rst

Note clang-format is not a style linter - it is a bulk code
reformatter that does waaaaaaay more than the .editorconfig
or .dir-locals does - our code is largely compliant with
the .editorconfig/.dir-locals rules, so it is good that we
ensure continued compliance by default.  clang-format is a
completely different situation as essentially none of our
code will be compliant today.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [RFC PATCH] Add qemu .clang-format

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 1370 bytes --]

Hi

On Wed, Aug 31, 2022 at 10:24 AM Wang, Lei <lei4.wang@intel.com> wrote:

>
> On 10/2/2015 1:30 AM, marcandre.lureau@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > clang-format is awesome to reflow your code according to qemu coding
> > style in an editor (in the region you modify).
> >
> > (note: clang-tidy should be able to add missing braces around
> > statements, but I haven't tried it, it's quite recent)
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >   .clang-format | 6 ++++++
> >   1 file changed, 6 insertions(+)
> >   create mode 100644 .clang-format
> >
> > diff --git a/.clang-format b/.clang-format
> > new file mode 100644
> > index 0000000..6422547
> > --- /dev/null
> > +++ b/.clang-format
> > @@ -0,0 +1,6 @@
> > +BasedOnStyle: LLVM
> > +IndentWidth: 4
> > +UseTab: Never
> > +BreakBeforeBraces: Linux
> > +AllowShortIfStatementsOnASingleLine: false
> > +IndentCaseLabels: false
>
> Hi, any progress on this? I also found a gist on GitHub which can be a
> reference:
> https://gist.github.com/elmarco/aa5e0b23567f46fb7f0e73cde586a0c1
>
>
Yes, that was a more complete configuration from me as well, but it is 5y
old already and might need some new updates.

Feel free to submit a new updated version.


-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 2260 bytes --]