[PATCH] D104044: [clang-format] Fix the issue of no empty line after namespace

MyDeveloperDay via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jun 11 10:40:45 PDT 2021


MyDeveloperDay added a comment.

My point being there is inconsistency between how different types of blocks of code are handled, and rather than trying to fix another corner case maybe we should take a more holistic approach, all these KeepEmptyLines and EmptyLineAfterXXX options and what you'll need in order to fix this issue are all addressing what is effectively the same issue, and that is that the addition and/or removal of empty lines is a little hit or miss depending on your combination and permutation of settings and the type of block

I personally would prefer we took a step back and asked ourselves if we are really facing a bug here or just different people desiring different functionality?

Whatever the rules are for an inner class, I don't particularly see they are much different for a class in a namespace (which I why I picked that example to demonstrate the point), we won't resolve that inconsistency in a way that will satisfy everyone without having a more powerful mechanism.

If you are thinking you want to just fix your bug then I'd be saying that it SHOULD remove the empty lines (including the one prior to the } // namespace MyLibrary, having said that I'm slightly struggling to understand why

  class Foo {
  
  
  
  
  class Bar {};
  };

isn't conforming to the setting of MaxEmptyLinesToKeep if I set it to 1 where

  namespace MyLibrary {
  
  class Tool {};
  }  // namespace MyLibrary

is

i.e. set MaxEmptyLinesToKeep  to 0 and

it gets formatted as:

  namespace MyLibrary {
  class Tool {};
  }  // namespace MyLibrary

We don't normally just review tests, but what were you thinking the fix should be?


Repository:
  rZORG LLVM Github Zorg

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104044/new/

https://reviews.llvm.org/D104044



More information about the cfe-commits mailing list