[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines
Yusuke Kadowaki via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Sun Sep 4 08:14:24 PDT 2022
yusuke-kadowaki added inline comments.
================
Comment at: clang/docs/ClangFormatStyleOptions.rst:3698
- QualifierOrder: ['inline', 'static', 'type', 'const']
+ QualifierOrder: ['inline', 'static' , 'type', 'const']
----------------
HazardyKnusperkeks wrote:
> Anyone knows where this comes from?
> You are using the script to generate the rst, right?
Yes, I'm using `dump_format_style.py`.
================
Comment at: clang/include/clang/Format/Format.h:154
/// \code
/// AlignConsecutiveMacros: AcrossEmptyLines
///
----------------
HazardyKnusperkeks wrote:
> 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. :)
Got it. Reverted for now.
================
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) {
----------------
HazardyKnusperkeks wrote:
> And accompanied by a short test. That should be everything to support the mixture of the options, right?
> That should be everything to support the mixture of the options, right?
I think so.
Done.
================
Comment at: clang/lib/Format/WhitespaceManager.cpp:984
+ }
+ else if (BreakBeforeNext || Newlines > NewLineThreshold ||
(ChangeMinColumn > MaxColumn || ChangeMaxColumn < MinColumn) ||
----------------
HazardyKnusperkeks wrote:
> Is this clang-format formatted?
I forgot that. Also formatted some other diffs.
================
Comment at: clang/unittests/Format/FormatTestComments.cpp:4262
/\
-/
+/
)",
----------------
HazardyKnusperkeks wrote:
> Please revert this.
Reverted.
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