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

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.


================
Comment at: clang/docs/ClangFormatStyleOptions.rst:865
+
+  * ``TCAS_DontAlign`` (in configuration: ``DontAlign``)
+    Don't align trailing comments.
----------------
yusuke-kadowaki wrote:
> 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.
Leave should mean do nothing.. I'm personally not a fan of DontAlign because obvious there should be a ' between the n and the t but I see we use it elsewhere

But actually now I see your DontAlign is effectively the as before (i.e. it will not add extra spaces) 

The point of Leave is what people have been asking for when we introduce a new option, that is we if possible add an option which means "Don't touch it at all"  i.e.  if I want to have

```
int a;           //  abc
int b;   /// bcd
int c;                                    // cde
```

Then so be it




================
Comment at: clang/include/clang/Format/Format.h:373
+  /// Different styles for aligning trailing comments.
+  enum TrailingCommentsAlignmentStyle : int8_t {
+    /// Don't align trailing comments.
----------------
yusuke-kadowaki wrote:
> 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.
I think its a bad idea to hijack a different option to do this..I don't think it means the same thing and I think what whilst it might work there will be someone somewhere who doesn't want it to behave like this and will call it out as a bug.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132131