yusuke-kadowaki marked 3 inline comments as done.
yusuke-kadowaki added inline comments.
================
Comment at: clang/docs/ClangFormatStyleOptions.rst:865
+
+ * ``TCAS_DontAlign`` (in configuration: ``DontAlign``)
+ Don't align trailing comments.
----------------
MyDeveloperDay wrote:
> Is Don'tAlign the same as Leave that other options support (for options it
> best if we try and use the same terminiology
>
> Always,Never,Leave (if that fits)
IMHO, `Leave` probably fits but `DontAlign` is more straightforward. Also
`DontAlign` is used for other alignment styles like `BracketAlignmentStyle` so
it would be more natural.
================
Comment at: clang/include/clang/Format/Format.h:373
+ /// Different styles for aligning trailing comments.
+ enum TrailingCommentsAlignmentStyle : int8_t {
+ /// Don't align trailing comments.
----------------
MyDeveloperDay wrote:
> all options have a life cycle
>
> bool -> enum -> struct
>
> One of the thing I ask you before, would we want to align across N empty
> lines, if ever so they I think
> we should go straight to a struct
> all options have a life cycle
I see your point. But we are checking `Style.MaxEmptyLinesToKeep` to determine
how many consecutive lines to align. So currently no need to specify it from
the option. You'll find the implementation below.
================
Comment at: clang/unittests/Format/FormatTestComments.cpp:2930
+ Style.ColumnLimit = 15;
+ EXPECT_EQ("int ab; // line\n"
+ "int a; // long\n"
----------------
MyDeveloperDay wrote:
> yusuke-kadowaki wrote:
> > MyDeveloperDay wrote:
> > > Why not verifyFormat here too and below?
> > To test the ColumnLimit behavior.
> pretty sure you can pass in a style
Style isn't a problem here. But when testing column limit, you want to see
before and after the line split into multiple lines. Original test cases of
AlignTrailingComments also use this EXPECT_EQ style not verifyFormat.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D132131/new/
https://reviews.llvm.org/D132131
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
