HazardyKnusperkeks added inline comments.
================
Comment at: clang/docs/ClangFormatStyleOptions.rst:865
+
+ * ``TCAS_DontAlign`` (in configuration: ``DontAlign``)
+ Don't align trailing comments.
----------------
MyDeveloperDay wrote:
> 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
>
>
Leave is a nice option, yes.
I think it is complementary to `Dont`.
But maybe if the option is called `AlignTrailingComments` one may interpret
`Leave` not as "don't touch the position of a comment at all" (what do we do,
if the comment is outside of the column limit?), but as "just don't touch them,
when they are somewhat aligned". Just a thought.
================
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:
> 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.
>
>
Since you are strictly against `Enabled` what to put into a struct?
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
