Page MenuHomePhabricator

[clang-format] use spaces for alignment of binary/ternary expressions with UT_AlignWithSpaces
ClosedPublic

Authored by fickert on Aug 9 2020, 3:48 AM.

Details

Summary

Use spaces to align binary and ternary expressions when using AlignOperands and UT_AlignWithSpaces.

Diff Detail

Event Timeline

fickert created this revision.Aug 9 2020, 3:48 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 9 2020, 3:48 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
fickert requested review of this revision.Aug 9 2020, 3:48 AM

This fixes an oversight in the new UT_AlignWithSpaces option (see D75034), which did not correctly identify the alignment of binary/ternary expressions.

curdeius added inline comments.
clang/lib/Format/ContinuationIndenter.cpp
1366–1371

You could factor out the part of the condition that doesn't concern AlignOperands to get rid of the repetition wrt. the previous if.

fickert updated this revision to Diff 284643.Aug 11 2020, 4:07 AM

Resolved repetitive conditions in the two consecutive ifs and added a small comment.

fickert marked an inline comment as done.Aug 11 2020, 4:08 AM
curdeius accepted this revision.Aug 11 2020, 4:13 AM

LGTM. Thank you for the patch!

This revision is now accepted and ready to land.Aug 11 2020, 4:13 AM

Thanks! I don't have push permissions to the repository so I cannot submit the commit myself (Maximilian Fickert <maximilian.fickert@gmail.com>).

Typz added a subscriber: Typz.Aug 11 2020, 4:25 AM

Could this also be causing https://bugs.llvm.org/show_bug.cgi?id=33896 ?

I'll try to see if changes something in that case...

Typz added a comment.Aug 11 2020, 4:33 AM

Could this also be causing https://bugs.llvm.org/show_bug.cgi?id=33896 ?

I'll try to see if changes something in that case...

Actually, I can confirm this is unrelated.

clang/lib/Format/ContinuationIndenter.cpp
1360

bad indent here : should be space-indented.

Yes I think it's unrelated, though this patch should fix the alignment in that example when using the UT_AlignWithSpaces option.

fickert added inline comments.Aug 11 2020, 4:42 AM
clang/lib/Format/ContinuationIndenter.cpp
1360

Can you clarify? This is formatted with the LLVM clang-format.

Typz added inline comments.Aug 11 2020, 4:51 AM
clang/lib/Format/ContinuationIndenter.cpp
1360

Phabricator shows some kind of >> symbol at the beginning of this line, which i interpreted as a TAB character, hence my comment.
But indeed no issue after looking at the raw file, sorry about this.