krasimir marked an inline comment as done.
krasimir added inline comments.
================
Comment at: clang/unittests/Format/FormatTest.cpp:6287
+ Style.BreakBeforeTernaryOperators = false;
+ verifyFormat("int x = aaaaaaaaaaaaaaa ? aaaaaaaaaaaaaaaaaa :\n"
+ " bbbb ? cccccccccccccccccc :\n"
----------------
sammccall wrote:
> aligning the question marks here is a bit weird (given DontAlign) but that's
> another patch.
>
> If we disable the question-column behavior with dontalign, this patch will be
> completely dead, right?
> May want to add a FIXME to remove in that case.
Added a FIXME. I think disabling question-column alignment with dontalign
indeed can make this patch obsolete.
================
Comment at: clang/unittests/Format/FormatTest.cpp:6287
+ Style.BreakBeforeTernaryOperators = false;
+ verifyFormat("int x = aaaaaaaaaaaaaaa ? aaaaaaaaaaaaaaaaaa :\n"
+ " bbbb ? cccccccccccccccccc :\n"
----------------
Typz wrote:
> krasimir wrote:
> > sammccall wrote:
> > > aligning the question marks here is a bit weird (given DontAlign) but
> > > that's another patch.
> > >
> > > If we disable the question-column behavior with dontalign, this patch
> > > will be completely dead, right?
> > > May want to add a FIXME to remove in that case.
> > Added a FIXME. I think disabling question-column alignment with dontalign
> > indeed can make this patch obsolete.
> I don't think this is so weird: even with DontAlign, there are other cases
> when some alignment is performed: for exemple when formatting tables in
> column.
>
> As I see it, ternary operator formatting is a similar case of 2D formatting,
> and while it needs indeed to respect the "general" line wrapping/indent mode
> (as per AlignOperands), it is OK to keep the alignment of the ternary
> operator themselves : otherwise, the whole "mode" for ternary operators need
> to be disabled and makes no sense.
>
> But anyway this is a different patch as you mentioned, and maybe some user of
> DontAlign can come up with a better approach for formatting ternary ops in
> that case.
Agree, this is more of a policy / preference on how we want these snippets to
look like with DontAlign. We should discuss it in more detail whenever we
attempt to refine this later. In a way, this touches back on djasper's comment
on D50078, which I believe was left unaddressed:
> I don't think the alignment of "?" and ":" (in the WhitespaceManager) should
> be always on. I think we'd need a flag for that part
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D82199/new/
https://reviews.llvm.org/D82199
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
