New issue
Advanced search Search tips

Issue 710164 link

Starred by 2 users

Issue metadata

Status: Untriaged
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

git cl format doesn't insert newlines in some cases

Project Member Reported by sky@chromium.org, Apr 10 2017

Issue description

In file foo_unittest.cc if I have:

#include "bar.h"
#include "foo.h"

Then git cl format reorders imports to:

#include "foo.h"
#include "bar.h"

There should be a newline between those two lines, e.g.:

#include "foo.h"

#include "bar.h"

 

Comment 1 by thakis@chromium.org, Apr 11 2017

Labels: clang-format
This is currently working as implemented: clang-format will reorder includes within continuous non-whitespace-separated #includes, but it won't do anything between blocks.

However, clang-format does have logic to try and guess the main header and bubble that to the top of the first block, and having it include a newline after that first line when it does that seems like a reasonable feature request. I'll ask for it upstream.

Comment 3 by thakis@chromium.org, May 12 2017

Cc: fdoray@chromium.org
 Issue 721767  has been merged into this issue.

Comment 4 by gab@chromium.org, May 12 2017

Can we turn that feature (bubbling main header to top) off in the meantime though? When mass refactoring it's uglier to end up with the main header on top of the block but not separate than to leave it in the state it was...

Comment 5 by thakis@chromium.org, May 12 2017

https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_Includes doesn't say anything about a newline, while it does mention putting the main .h first (to ensure it's stand-alone). So I think the current behavior is better than not bubbling the main header to the top.

Comment 6 by thakis@chromium.org, May 12 2017

I.e. the reasons for bubbling aren't just stylistic but also technical.

Comment 7 by gab@chromium.org, May 12 2017

Got it, so in the meantime then, we won't bother adding a newline in mass refactors that make this automatic move as that's just tedious and mostly a waste of engineering time.

Comment 8 by thakis@chromium.org, May 12 2017

Yup, makes sense :-)

Sign in to add a comment