[PATCH] D12362: [clang-format] Add support of consecutive declarations alignment

Daniel Jasper via cfe-commits cfe-commits at lists.llvm.org
Tue Sep 29 09:31:33 PDT 2015


djasper added inline comments.

================
Comment at: unittests/Format/FormatTest.cpp:8699
@@ +8698,3 @@
+                   Alignment));
+  Alignment.AlignConsecutiveAssignments = true;
+  verifyFormat("float      something = 2000;\n"
----------------
berenm wrote:
> djasper wrote:
> > Can you add a case (unless I missed it) where aligning both consecutive assignments and consecutive declarations exceed the column limit? What should happen in that case? I am thinking of something like:
> > 
> >   int loooooooooooooooongName = 1;
> >   LoooooooooooongType i = bbbbbbbbbbbbbbbbbbbbbbb;
> AFAIR, the alignment doesn't work very well with the column limit at the moment. This is already true wrt the assignment alignment. The column limit is enforced before the alignment is done and aligning variable names and / or assignment will expand beyond that limit.
> 
> I will add the test case but I haven't tried to fix this issue yet.
> 
> Should test cases check the current behaviour or the ideal expected behaviour (that doesn't work) ?
Ah interesting. Then I am ok with submitting this as is. However, the issue should be fixed for both. It is fine to do the alignment after doing the overall layout, but we should then simply not align if alignment would violate the column limit. This is already done for trailing comments.

Please add a comment (FIXME) about this somewhere.


http://reviews.llvm.org/D12362





More information about the cfe-commits mailing list