[PATCH] D82199: [clang-format] restore indent in conditionals when AlignOperands is DontAlign

[Krasimir Georgiev via Phabricator via cfe-commits]

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