[PATCH] clang-format: Set IndentWrappedFunctionNames false

[PATCH] clang-format: Set IndentWrappedFunctionNames false

[Jason Gunthorpe]

The true option causes this indenting for functions:

static struct something_very_very_long *
    function(void *arg)
{

While a quick survey suggests that the usual Linux fallback is the GNU
style:

static struct something_very_very_long *
function(void *arg)
{

Eg as seen in:

 kernel/cpu.c
 kernel/fork.c

and other places.

Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
---
 .clang-format | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Thanks for making this Miguel!

I've been using clang-format for years with the kernel and noticed it
was performing just a little different than I would have expected.

AFAIK the above describes the more common choice in the code base?

Not sure who's tree this is supposed to go through.. Andrew I guess?

diff --git a/.clang-format b/.clang-format
index faffc0d5af4eeb..1d5da22e0ba50c 100644
--- a/.clang-format
+++ b/.clang-format
@@ -382,7 +382,7 @@ IncludeIsMainRegex: '(Test)?$'
 IndentCaseLabels: false
 #IndentPPDirectives: None # Unknown to clang-format-5.0
 IndentWidth: 8
-IndentWrappedFunctionNames: true
+IndentWrappedFunctionNames: false
 JavaScriptQuotes: Leave
 JavaScriptWrapImports: true
 KeepEmptyLinesAtTheStartOfBlocks: false
-- 
2.17.0

Re: [PATCH] clang-format: Set IndentWrappedFunctionNames false

[Joe Perches]

On Wed, 2018-06-06 at 14:15 -0600, Jason Gunthorpe wrote:
> The true option causes this indenting for functions:
> 
> static struct something_very_very_long *
>     function(void *arg)
> {
> 
> While a quick survey suggests that the usual Linux fallback is the GNU
> style:
> 
> static struct something_very_very_long *
> function(void *arg)
> 
[]
> AFAIK the above describes the more common choice in the code base?
> 
> Not sure who's tree this is supposed to go through.. Andrew I guess?

I believe so yes.

I expect there will be more refinements to .clang-formata
as perhaps more people experiment with it too.

Acked-by: Joe Perches <joe@perches.com>

> diff --git a/.clang-format b/.clang-format
> index faffc0d5af4eeb..1d5da22e0ba50c 100644
> --- a/.clang-format
> +++ b/.clang-format
> @@ -382,7 +382,7 @@ IncludeIsMainRegex: '(Test)?$'
>  IndentCaseLabels: false
>  #IndentPPDirectives: None # Unknown to clang-format-5.0
>  IndentWidth: 8
> -IndentWrappedFunctionNames: true
> +IndentWrappedFunctionNames: false
>  JavaScriptQuotes: Leave
>  JavaScriptWrapImports: true
>  KeepEmptyLinesAtTheStartOfBlocks: false

Re: [PATCH] clang-format: Set IndentWrappedFunctionNames false

[Jason Gunthorpe]

On Wed, Jun 06, 2018 at 01:18:59PM -0700, Joe Perches wrote:
> On Wed, 2018-06-06 at 14:15 -0600, Jason Gunthorpe wrote:
> > The true option causes this indenting for functions:
> > 
> > static struct something_very_very_long *
> >     function(void *arg)
> > {
> > 
> > While a quick survey suggests that the usual Linux fallback is the GNU
> > style:
> > 
> > static struct something_very_very_long *
> > function(void *arg)
> > 
> []
> > AFAIK the above describes the more common choice in the code base?
> > 
> > Not sure who's tree this is supposed to go through.. Andrew I guess?
> 
> I believe so yes.
> 
> I expect there will be more refinements to .clang-formata
> as perhaps more people experiment with it too.
> 
> Acked-by: Joe Perches <joe@perches.com>

Thanks Joe,

I saw your note in the original mailing-list thread and just wanted to
say the way I use clang-format is editor based.

I have a hotkey that triggers clang-format to reformat the current
statement, or current hi-lighted region. If I don't like it, then
'undo' puts it all back for manual adjustment..

This way I am selective of where I apply it, and I find it is
something like 95% of the time good, IHMO.

Big time saver for wrapping long lines, moving code around (eg
indent,de-indent) and other mundane daily tasks.

Cheers,
Jason

Re: [PATCH] clang-format: Set IndentWrappedFunctionNames false

[Joe Perches]

On Wed, 2018-06-06 at 14:25 -0600, Jason Gunthorpe wrote:
> I saw your note in the original mailing-list thread and just wanted to
> say the way I use clang-format is editor based.
> 
> I have a hotkey that triggers clang-format to reformat the current
> statement, or current hi-lighted region. If I don't like it, then
> 'undo' puts it all back for manual adjustment..
> 
> This way I am selective of where I apply it, and I find it is
> something like 95% of the time good, IHMO.
> 
> Big time saver for wrapping long lines, moving code around (eg
> indent,de-indent) and other mundane daily tasks.

Cool for you.

I do most of that via emacs indent-region but perhaps
for rewrapping content and such, clang-format-region
could be even better.

I'll give it a go one day.

cheers, Joe

Re: [PATCH] clang-format: Set IndentWrappedFunctionNames false

[Miguel Ojeda]

On Wed, Jun 6, 2018 at 10:18 PM, Joe Perches <joe@perches.com> wrote:
> On Wed, 2018-06-06 at 14:15 -0600, Jason Gunthorpe wrote:
>> The true option causes this indenting for functions:
>>
>> static struct something_very_very_long *
>>     function(void *arg)
>> {
>>
>> While a quick survey suggests that the usual Linux fallback is the GNU
>> style:
>>
>> static struct something_very_very_long *
>> function(void *arg)
>>
> []
>> AFAIK the above describes the more common choice in the code base?
>>
>> Not sure who's tree this is supposed to go through.. Andrew I guess?
>
> I believe so yes.
>
> I expect there will be more refinements to .clang-formata
> as perhaps more people experiment with it too.

Yes, indeed! The version I committed was an initial approximation --
don't expect it to be the "final" or "official" one!

Cheers,
Miguel

>
> Acked-by: Joe Perches <joe@perches.com>
>
>> diff --git a/.clang-format b/.clang-format
>> index faffc0d5af4eeb..1d5da22e0ba50c 100644
>> --- a/.clang-format
>> +++ b/.clang-format
>> @@ -382,7 +382,7 @@ IncludeIsMainRegex: '(Test)?$'
>>  IndentCaseLabels: false
>>  #IndentPPDirectives: None # Unknown to clang-format-5.0
>>  IndentWidth: 8
>> -IndentWrappedFunctionNames: true
>> +IndentWrappedFunctionNames: false
>>  JavaScriptQuotes: Leave
>>  JavaScriptWrapImports: true
>>  KeepEmptyLinesAtTheStartOfBlocks: false

Re: [PATCH] clang-format: Set IndentWrappedFunctionNames false

[Miguel Ojeda]

On Wed, Jun 6, 2018 at 10:15 PM, Jason Gunthorpe <jgg@mellanox.com> wrote:
> The true option causes this indenting for functions:
>
> static struct something_very_very_long *
>     function(void *arg)
> {
>
> While a quick survey suggests that the usual Linux fallback is the GNU
> style:
>
> static struct something_very_very_long *
> function(void *arg)
> {
>
> Eg as seen in:
>
>  kernel/cpu.c
>  kernel/fork.c
>
> and other places.
>
> Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
> ---
>  .clang-format | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> Thanks for making this Miguel!

You're welcome! It is great to see people already using it. :-)

>
> I've been using clang-format for years with the kernel and noticed it
> was performing just a little different than I would have expected.
>
> AFAIK the above describes the more common choice in the code base?

Could be -- I took a look at several places and to the official
guidelines; but I am sure I may have missed some things :-) For some
options where I was unsure, I ran it through several "popular"/common
folders and see which one generated a smaller git diff -- maybe you
can try that and see what you get (also, it would be great if you can
try some others outside kernel/*).

>
> Not sure who's tree this is supposed to go through.. Andrew I guess?

It went through Andre the first time. In any case, I think it is fine
to send it through any one.

Cheers,
Miguel

>
> diff --git a/.clang-format b/.clang-format
> index faffc0d5af4eeb..1d5da22e0ba50c 100644
> --- a/.clang-format
> +++ b/.clang-format
> @@ -382,7 +382,7 @@ IncludeIsMainRegex: '(Test)?$'
>  IndentCaseLabels: false
>  #IndentPPDirectives: None # Unknown to clang-format-5.0
>  IndentWidth: 8
> -IndentWrappedFunctionNames: true
> +IndentWrappedFunctionNames: false
>  JavaScriptQuotes: Leave
>  JavaScriptWrapImports: true
>  KeepEmptyLinesAtTheStartOfBlocks: false
> --
> 2.17.0
>

Re: [PATCH] clang-format: Set IndentWrappedFunctionNames false

[Miguel Ojeda]

On Wed, Jun 6, 2018 at 10:25 PM, Jason Gunthorpe <jgg@mellanox.com> wrote:
> On Wed, Jun 06, 2018 at 01:18:59PM -0700, Joe Perches wrote:
>> On Wed, 2018-06-06 at 14:15 -0600, Jason Gunthorpe wrote:
>> > The true option causes this indenting for functions:
>> >
>> > static struct something_very_very_long *
>> >     function(void *arg)
>> > {
>> >
>> > While a quick survey suggests that the usual Linux fallback is the GNU
>> > style:
>> >
>> > static struct something_very_very_long *
>> > function(void *arg)
>> >
>> []
>> > AFAIK the above describes the more common choice in the code base?
>> >
>> > Not sure who's tree this is supposed to go through.. Andrew I guess?
>>
>> I believe so yes.
>>
>> I expect there will be more refinements to .clang-formata
>> as perhaps more people experiment with it too.
>>
>> Acked-by: Joe Perches <joe@perches.com>
>
> Thanks Joe,
>
> I saw your note in the original mailing-list thread and just wanted to
> say the way I use clang-format is editor based.
>
> I have a hotkey that triggers clang-format to reformat the current
> statement, or current hi-lighted region. If I don't like it, then
> 'undo' puts it all back for manual adjustment..
>
> This way I am selective of where I apply it, and I find it is
> something like 95% of the time good, IHMO.
>
> Big time saver for wrapping long lines, moving code around (eg
> indent,de-indent) and other mundane daily tasks.

Indeed, I agree that currently this is the best way to use it (until
and if we get full support for the kernel style in clang). In the
documentation I wrote I mentioned this point. It can also be useful
file-wide to quickly spot style issues: apply clang-format - inspect
git diff - reset back.

Cheers,
Miguel

>
> Cheers,
> Jason