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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
