HazardyKnusperkeks added inline comments.

================
Comment at: clang/docs/ClangFormatStyleOptions.rst:797
+    AlignConsecutiveMacros:
+      Enabled: true
+      AcrossEmptyLines: true
----------------
MyDeveloperDay wrote:
> why do we need Enabled?
> 
> isn't it just
> 
> ```
> false:
> AcrossEmptyLines: false
> AcrossComments: false
> 
> true:
> AcrossEmptyLines: true
> AcrossComments: true
> ```
The struct is a bit older. And we need `Enabled`, one might wish to align only 
really consecutive comments, as the option right now does. (Same for macros, 
assignments, etc..)


================
Comment at: clang/lib/Format/Format.cpp:649
     IO.mapOptional("AlignOperands", Style.AlignOperands);
-    IO.mapOptional("AlignTrailingComments", Style.AlignTrailingComments);
     IO.mapOptional("AllowAllArgumentsOnNextLine",
----------------
MyDeveloperDay wrote:
> HazardyKnusperkeks wrote:
> > MyDeveloperDay wrote:
> > > you can't remove an option, otherwise you'll break everyones .clang-format
> > That's not correct. We have done it:
> > D108752 -> D108882 -> D127390
> > 
> > You can remove (and in this case should), but you still need to parse it 
> > and act accordingly. Which is done as far as I can see.
> sorry thats what I meant, you can remove it, but you have to make it turn on 
> the correct new style that keeps exactly the old user case, and you can't 
> remove it from the configuration parsing otherwise anyone who has it in their 
> .clang-format is immediately broken with an unknown option.
> 
> to be honest this is an annoyance for introducing new features, which at some 
> point I'd like to drop, you can't have a new option which is not read
> 
> For me, when we introduced new languages, or new features like InsertBraces 
> etc.. I want to put it in my .clang-format even though other people they 
> aren't using a version that uses it. (becuase it won't impact them), i.e. it 
> won't add braces or correct QualifierOrder that doesn't bother me
> 
Do you disagree that it actually is parsed?

But that compatibility parsing has nothing to do with ignoring unknown (new) 
options.


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

Reply via email to