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

This revision was automatically updated to reflect the committed changes. Closed by commit rGe8111502d869: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation (authored by MyDeveloperDay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

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

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

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. This LGTM, thank you Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75034/new/ https://reviews.llvm.org/D75034

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

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

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

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

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] D75034: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation

MyDeveloperDay added a comment. I want to believe that it was a mistake on the original developers part, but I just can't tell. A search of github shows people using this in their repos https://github.com/search?q=ForContinuationAndIndentation=Code I almost feel unless the original author can

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

MyDeveloperDay added a comment. is there overlap here D76197: clang-format: Use block indentation for braced initializations CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75034/new/ https://reviews.llvm.org/D75034

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

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

mitchell-stellar added a subscriber: mxbOctasic. mitchell-stellar added a comment. Digging through the history, it seems this behavior was explicitly desired: https://reviews.llvm.org/D19028. @mxbOctasic was the original author and perhaps could comment. Regardless, I think there should be a

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

MyDeveloperDay added a comment. I tend to agree with your assessment, I think its worth waiting for some others to comment. Comment at: clang/unittests/Format/FormatTest.cpp:10293 Tab)); - EXPECT_EQ("/*\n" -"\t a\t\tcomment\n"

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

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

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

MyDeveloperDay added subscribers: djasper, klimek. MyDeveloperDay added a comment. > 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 For example I can't understand with the

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

MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:10293 Tab)); - EXPECT_EQ("/*\n" -"\t a\t\tcomment\n" Sorry this isn't clear to be which test this is a duplicate of, as this is an

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

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

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

MyDeveloperDay added a comment. you need to add any diff as a full context diff (normally by adding -U 999) to the diff command Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75034/new/ https://reviews.llvm.org/D75034

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

MyDeveloperDay added a comment. If you are changing unit tests then I'm nervous, imagine if this many tests change what the impact is on a large code base. I think we need to understand more about why you are making this change and why you think its ok to change/remove existing tests which

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

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

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