[libcamera-devel] clang-format: Set ColumnLimit to 80

429 diff mbox series

Our maximum column limit is 120 characters, but we will still aim for an 80
line boundary where it is possible. The 120 is a limit for exceptions.

Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 .clang-format | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Hi Laurent,

On 28/01/2019 11:26, Kieran Bingham wrote:
> Our maximum column limit is 120 characters, but we will still aim for an 80
> line boundary where it is possible. The 120 is a limit for exceptions.
> 

It seems that 80 is the clang-format default - so the fact that this
ColumnLimit is currently set to 0 puzzles me. (This means that
clang-format will not adjust the line length, unless it breaks some
other rule)

Perhaps it should at least be set to 120 ?

Was  "ColumnLimit 0"  a default in the file? or an intentional setting?



> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
>  .clang-format | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/.clang-format b/.clang-format
> index 17eb095bde71..59a61d050e3d 100644
> --- a/.clang-format
> +++ b/.clang-format
> @@ -52,7 +52,7 @@ BreakConstructorInitializersBeforeComma: false
>  #BreakConstructorInitializers: BeforeComma # Unknown to clang-format-4.0
>  BreakAfterJavaFieldAnnotations: false
>  BreakStringLiterals: false
> -ColumnLimit: 0
> +ColumnLimit: 80
>  CommentPragmas: '^ IWYU pragma:'
>  #CompactNamespaces: false # Unknown to clang-format-4.0
>  ConstructorInitializerAllOnOneLineOrOnePerLine: false
>

Hi Kieran,

On Mon, Jan 28, 2019 at 11:40:05AM +0000, Kieran Bingham wrote:
> On 28/01/2019 11:26, Kieran Bingham wrote:
> > Our maximum column limit is 120 characters, but we will still aim for an 80
> > line boundary where it is possible. The 120 is a limit for exceptions.
> 
> It seems that 80 is the clang-format default - so the fact that this
> ColumnLimit is currently set to 0 puzzles me. (This means that
> clang-format will not adjust the line length, unless it breaks some
> other rule)
> 
> Perhaps it should at least be set to 120 ?
> 
> Was  "ColumnLimit 0"  a default in the file? or an intentional setting?

I tried setting it to 120, but clang-format merged lines smaller than
120 columns to get as close as possible to the limit, which was clearly
not a good idea. I wish there was an option to set 80 as the target and
120 as an exception, but that doesn't seem to be the case. I went for 0
to avoid both automatic reformatting to 120 and avoiding numerous
warnings (as we have quite a few lines that go over 80 columns). I'm
fine experimenting with the limit set to 80, but I fear it may result in
developers breaking lines when they shouldn't.

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > ---
> >  .clang-format | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/.clang-format b/.clang-format
> > index 17eb095bde71..59a61d050e3d 100644
> > --- a/.clang-format
> > +++ b/.clang-format
> > @@ -52,7 +52,7 @@ BreakConstructorInitializersBeforeComma: false
> >  #BreakConstructorInitializers: BeforeComma # Unknown to clang-format-4.0
> >  BreakAfterJavaFieldAnnotations: false
> >  BreakStringLiterals: false
> > -ColumnLimit: 0
> > +ColumnLimit: 80
> >  CommentPragmas: '^ IWYU pragma:'
> >  #CompactNamespaces: false # Unknown to clang-format-4.0
> >  ConstructorInitializerAllOnOneLineOrOnePerLine: false