tinloaf marked 5 inline comments as done.
tinloaf added inline comments.
================
Comment at: clang/include/clang/Format/Format.h:130
+ /// \endcode
+ ACA_AcrossComments
+ };
----------------
MyDeveloperDay wrote:
> tinloaf wrote:
> > MyDeveloperDay wrote:
> > > Is there a case for aligning over empty lines and comments?
> > >
> > > ```
> > > int a = 5;
> > >
> > > /* comment */
> > > int oneTwoThree = 123;
> > > ```
> > Not sure I understand what you mean. Currently, the Option `AcrossComments`
> > includes 'across empty lines'. So, there currently is no case for "across
> > comments, but not across empty lines". I'm not sure if that is really
> > something people want to do. Do you think so? I can add it.
> I could see a case where you might want to begin a new "alignment group" by
> leaving a blank line.
>
> ```
> /* align these 3 */
> int a = 5;
> /* align these 3 */
> int b = 6;
> /* align these 3 */
> int c = 7;
>
> /* align these 2 which are longer */
> int d = 5
> /* align these 2 which are longer */
> int oneTwoThree = 123;
> ```
Yes, good point. I've added the option `AlignAcrossEmptyLinesAndComments`, and
made `AlignAcrossComments` do what the name suggests.
================
Comment at: clang/lib/Format/WhitespaceManager.cpp:367
+ unsigned StartAt, bool AcrossEmpty = false,
+ bool AcrossComments = false) {
unsigned MinColumn = 0;
----------------
MyDeveloperDay wrote:
> could this be an enum?
>
> ```
> enum {
> None
> AcrossEmptyLines,
> AcrossComments,
> AcrossEmptyLinesAndComments,
> }
> ```
Yes, that's probably cleaner, though that means I need a rather ugly `switch`
statement to convert one enum into the other where `AlignTokens` is called.
Maybe one could address this with some template magic, but that's probably not
less ugly than the switch statement.
================
Comment at: clang/lib/Format/WhitespaceManager.cpp:421
// matching token, the sequence ends here.
- if (Changes[i].NewlinesBefore > 1 || !FoundMatchOnLine)
+ if (((Changes[i].NewlinesBefore > 1) && (!AcrossEmpty)) ||
+ (!FoundMatchOnLine && (!(LineIsComment && AcrossComments))))
----------------
curdeius wrote:
> Nit: unnecessary parentheses around !AcrossEmpty.
> Same around !(LineIsComment && AcrossComments).
> Maybe you might factor out a bool variable for this condition?
Good point. I factored this out into two booleans, which should improve
readability.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D93986/new/
https://reviews.llvm.org/D93986
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
