[PATCH] D35955: clang-format: Add preprocessor directive indentation

Mark Zeren via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Aug 23 14:27:26 PDT 2017


mzeren-vmw added inline comments.


================
Comment at: lib/Format/ContinuationIndenter.cpp:383
+      Current.Previous->is(tok::hash) && State.FirstIndent > 0) {
+    // subtract 1 so indent lines up with non-preprocessor code
+    Spaces += State.FirstIndent;
----------------
djasper wrote:
> euhlmann wrote:
> > djasper wrote:
> > > Same here and use full sentences.
> > > 
> > > Also, this does not seem to be what the example in the style option suggests and I'd rather not do it (subtracting 1).
> > I apologize, the style option documentation was a typo. The patch summary has the intended behavior, and I mentioned there that I count the hash as a column. Part of the reasoning for this is because it would look the same visually with spaces or tabs.
> Do you know of a coding style that writes something about this? I think the similarity of spaces vs. tabs is not a strong reason because a source file will either use one or the other. To me:
> 
>   #if a == 1
>   # define X
>   # if b == 2
>   #   define Y
>   ...
> 
> Would look weird. I'd prefer if we kept this simpler and more consistent.
@djasper I just noticed that the [[ https://google.github.io/styleguide/cppguide.html#Preprocessor_Directives | Google Style Guide  ]] has:
```
#if DISASTER_PENDING      // Correct -- Starts at beginning of line
    DropEverything();
# if NOTIFY               // OK but not required -- Spaces after #
    NotifyClient();
# endif
#endif
```
Note the single space in `# if NOTIFY`. Can we correct the Guide to match what we have here?



https://reviews.llvm.org/D35955





More information about the cfe-commits mailing list