[PATCH] D75034: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation

2020-02-24 Thread Maximilian Fickert via Phabricator via cfe-commits
fickert created this revision. fickert added reviewers: clang-format, MyDeveloperDay, krasimir. fickert added projects: clang-format, clang-tools-extra. Herald added a project: clang. Herald added a subscriber: cfe-commits. Use spaces instead of tabs for alignment with

[PATCH] D75034: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation

2020-02-24 Thread Maximilian Fickert via Phabricator via cfe-commits
fickert added a comment. I added an `IsAligned` flag to `ParenState` and `Change` to track if an indent was created for alignment. I adapted the corresponding test cases where alignments were previously done with tabs, including a test case where leading whitespace in a comment is interpreted

[PATCH] D75034: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation

2020-02-24 Thread Maximilian Fickert via Phabricator via cfe-commits
fickert updated this revision to Diff 246176. fickert marked an inline comment as done. fickert added a comment. uploaded full context diff CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75034/new/ https://reviews.llvm.org/D75034 Files: clang/lib/Format/BreakableToken.cpp

[PATCH] D75034: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation

2020-02-24 Thread Maximilian Fickert via Phabricator via cfe-commits
fickert added a comment. I understand the caution around changing the unit tests. The tests I modified assumed that alignment would be made with mixed tabs and spaces, which is arguably not the expected behavior with `UT_ForContinuationAndIndentation` (see

[PATCH] D75034: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation

2020-02-24 Thread Maximilian Fickert via Phabricator via cfe-commits
fickert marked an inline comment as done. fickert added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:10293 Tab)); - EXPECT_EQ("/*\n" -"\t a\t\tcomment\n" MyDeveloperDay wrote: > Sorry this isn't clear

[PATCH] D75034: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation

2020-02-24 Thread Maximilian Fickert via Phabricator via cfe-commits
fickert added a comment. In D75034#1889107 , @MyDeveloperDay wrote: > > Is this intentional, or is it an oversight? > > This is the key question, which I'm not sure how we can answer without the > help of the original authors @djasper and @klimek > >

[PATCH] D75034: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation

2020-04-13 Thread Maximilian Fickert via Phabricator via cfe-commits
fickert updated this revision to Diff 256938. fickert added a comment. rebased, fixed two unit tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75034/new/ https://reviews.llvm.org/D75034 Files: clang/docs/ClangFormatStyleOptions.rst

[PATCH] D75034: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation

2020-03-28 Thread Maximilian Fickert via Phabricator via cfe-commits
fickert updated this revision to Diff 253327. fickert added a comment. I moved the changed behavior to a new option (`UT_AlignWithSpaces`), and added corresponding unit tests by copying and adapting the tests for `UT_ForContinuationAndIndentation`. Repository: rG LLVM Github Monorepo

[PATCH] D75034: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation

2020-03-28 Thread Maximilian Fickert via Phabricator via cfe-commits
fickert added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:10146 "\t int i;\n" "}")); This test seems to be missing the tab configuration (`Tab`) as an argument to `format`, and the same test with

[PATCH] D75034: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation

2020-03-28 Thread Maximilian Fickert via Phabricator via cfe-commits
fickert updated this revision to Diff 253349. fickert added a comment. Added documentation and missing test cases for 0-width tabs. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75034/new/ https://reviews.llvm.org/D75034 Files:

[PATCH] D75034: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation

2020-04-02 Thread Maximilian Fickert via Phabricator via cfe-commits
fickert added a comment. Great, thank you! I do not have push access to the repository, so someone else would need to commit this for me: Maximilian Fickert Should I add the fix for the test cases missing the tab configuration (see my last inline comment:

[PATCH] D75034: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation

2020-03-17 Thread Maximilian Fickert via Phabricator via cfe-commits
fickert added a comment. From the original commit, it does seem that this behavior was desired. However, from the name of the option and description in the documentation ("Use tabs only for line continuation and indentation"), I would expect it to use tabs only for indentation and

[PATCH] D75034: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation

2020-03-24 Thread Maximilian Fickert via Phabricator via cfe-commits
fickert added a comment. In D75034#1938626 , @MyDeveloperDay wrote: > is there overlap here D76197: clang-format: Use block indentation for braced > initializations I think these changes are unrelated. CHANGES SINCE

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

2020-08-09 Thread Maximilian Fickert via Phabricator via cfe-commits
fickert added a comment. This fixes an oversight in the new `UT_AlignWithSpaces` option (see D75034 ), which did not correctly identify the alignment of binary/ternary expressions. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

2020-08-09 Thread Maximilian Fickert via Phabricator via cfe-commits
fickert created this revision. fickert added reviewers: clang-format, MyDeveloperDay. fickert added projects: clang-format, clang-tools-extra. Herald added a project: clang. Herald added a subscriber: cfe-commits. fickert requested review of this revision. Use spaces to align binary and ternary

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

2020-08-11 Thread Maximilian Fickert via Phabricator via cfe-commits
fickert added a comment. Thanks! I don't have push permissions to the repository so I cannot submit the commit myself (Maximilian Fickert ). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85600/new/ https://reviews.llvm.org/D85600

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

2020-08-11 Thread Maximilian Fickert via Phabricator via cfe-commits
fickert updated this revision to Diff 284643. fickert added a comment. Resolved repetitive conditions in the two consecutive `if`s and added a small comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85600/new/ https://reviews.llvm.org/D85600

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

2020-08-11 Thread Maximilian Fickert via Phabricator via cfe-commits
fickert added inline comments. Comment at: clang/lib/Format/ContinuationIndenter.cpp:1360 + if (Style.AlignOperands == FormatStyle::OAS_AlignAfterOperator) +NewParenState.UnindentOperator = true; + // Mark indentation as alignment if the expression is aligned.

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

2020-08-11 Thread Maximilian Fickert via Phabricator via cfe-commits
fickert added a comment. Yes I think it's unrelated, though this patch should fix the alignment in that example when using the `UT_AlignWithSpaces` option. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85600/new/ https://reviews.llvm.org/D85600