[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

[Yusuke Kadowaki via Phabricator via cfe-commits]

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