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

Björn Schäpers via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Sep 1 12:44:30 PDT 2022


HazardyKnusperkeks added a comment.

In my opinion we are nearly done.



================
Comment at: clang/docs/ClangFormatStyleOptions.rst:3698
 
-      QualifierOrder: ['inline', 'static', 'type', 'const']
+      QualifierOrder: ['inline', 'static' , 'type', 'const']
 
----------------
Anyone knows where this comes from?
You are using the script to generate the rst, right?


================
Comment at: clang/include/clang/Format/Format.h:154
   /// \code
   ///   AlignConsecutiveMacros: AcrossEmptyLines
   ///
----------------
yusuke-kadowaki wrote:
> HazardyKnusperkeks wrote:
> > The change/addition has to be here, since here it directly states `AlignConsecutiveMacros`.
> What are you meaning to say here? I thought saying `AlignConsecutiveStyle` is used for multiple options is what we are trying to do.
> 
> Should we say something like, 
> > Here AlignConsecutiveMacros is used as an example, but in practice AlignConsecutiveStyle is also used with other options.
> in this place?
The example mentions explicitly only `AlignConsecutiveMacros` which is a bit misleading if you are only looking at the documentation of `AlignConsecutiveAssignments` for example.

This is not your fault, and I'm fine with ignoring it here. A separate patch to fix that is welcome. :)


================
Comment at: clang/lib/Format/WhitespaceManager.cpp:930
   unsigned Newlines = 0;
+  unsigned int NewLineThreshold = Style.AlignConsecutiveTrailingComments.AcrossEmptyLines ? 2 : 1;
   for (unsigned i = 0, e = Changes.size(); i != e; ++i) {
----------------
And accompanied by a short test. That should be everything to support the mixture of the options, right?


================
Comment at: clang/lib/Format/WhitespaceManager.cpp:984
+    }
+    else if (BreakBeforeNext || Newlines > NewLineThreshold ||
                (ChangeMinColumn > MaxColumn || ChangeMaxColumn < MinColumn) ||
----------------
Is this clang-format formatted?


================
Comment at: clang/unittests/Format/FormatTestComments.cpp:4262
 /\
-/ 
+/
   )",
----------------
Please revert this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132131



More information about the cfe-commits mailing list