[PATCH] D136154: Fix the continuation indenter

hel-ableton created this revision. hel-ableton added reviewers: JonasToth, MyDeveloperDay, owenpan. hel-ableton added a project: clang-format. Herald added a project: All. hel-ableton requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. When Sty

[PATCH] D136154: [clang-format] Fix the continuation indenter

hel-ableton updated this revision to Diff 468793. hel-ableton added a comment. Sorry about the plus sign! I'm fairly unfamiliar with your process, so something between making a diff and making a patch must have gone wrong. About the brackets, I was wondering why there weren't any, but didn't see

[PATCH] D136154: [clang-format] Fix the continuation indenter

hel-ableton added a comment. In D136154#3867328 , @MyDeveloperDay wrote: > Can we log a GitHub issue I can’t see what you are trying to fix I'm sorry if this is unclear. The background to this is that our repository is currently formatted using clang-f

[PATCH] D136154: [clang-format] Fix the continuation indenter

hel-ableton added a comment. In D136154#3867328 , @MyDeveloperDay wrote: > Can we log a GitHub issue I can’t see what you are trying to fix Without the fix, the arguments to the Base class would be on the same level as the Base class itself. But this a

[PATCH] D136154: [clang-format] Fix the continuation indenter

hel-ableton added a comment. > must have been introduced somewhere between 7.1.0 and 10.0.0 In fact, someone in our team thought that this must have been introduced exactly with this commit: https://github.com/llvm/llvm-project/commit/4636debc271f09f730697ab6873137a657c828f9 CHANGES SINCE LAS

[PATCH] D136154: [clang-format] Fix the continuation indenter

hel-ableton added a comment. Adding two inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:6599-6601 + Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak; + Style.BinPackParameters = false; + Style.ContinuationIndentWidth = 2; Hazardy

[PATCH] D136154: [clang-format] Fix the continuation indenter

hel-ableton added a comment. In D136154#3867935 , @MyDeveloperDay wrote: > You've dropped the test on your most recent updated Oh, that's why it appeared from the diff. Apologies again, I'm really unfamiliar with your process. I guess it puzzles me why

[PATCH] D136154: [clang-format] Fix the continuation indenter

hel-ableton updated this revision to Diff 468874. hel-ableton added a comment. This should bring back the formerly introduced unit test. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136154/new/ https://reviews.llvm.org/D136154 Files: clang/lib/Format/ContinuationIndenter.cpp clang

[PATCH] D136154: [clang-format] Fix the continuation indenter

hel-ableton added a comment. > The problem as I see it was that the original bug, highly constrained the > cases where "CurrentState.LastSpace = State.Column;" to one particular style > (which if it happens to be your style great but not if its not. You mean the original bugfix was already unne

[PATCH] D136154: [clang-format] Fix the continuation indenter

hel-ableton updated this revision to Diff 468878. hel-ableton added a comment. Using verifyFormat() now instead of EXPECT_EQ() CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136154/new/ https://reviews.llvm.org/D136154 Files: clang/lib/Format/ContinuationIndenter.cpp clang/unittests

[PATCH] D136154: [clang-format] Fix the continuation indenter

hel-ableton marked an inline comment as done. hel-ableton added a comment. In D136154#3867968 , @MyDeveloperDay wrote: > Doesn't something like this achieve the same > >CurrentState.Indent = State.Column; >CurrentState.LastSpace = State.

[PATCH] D136154: [clang-format] Fix the continuation indenter

hel-ableton updated this revision to Diff 468907. hel-ableton added a comment. Fixed missing newline. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136154/new/ https://reviews.llvm.org/D136154 Files: clang/lib/Format/ContinuationIndenter.cpp clang/unittests/Format/FormatTest.cpp

[PATCH] D136154: [clang-format] Fix the continuation indenter

hel-ableton marked an inline comment as done. hel-ableton added a comment. > I do, but I'd like to hear @owenpan and @HazardyKnusperkeks view.. (each of > us is better in different part of clang-format, and I value their opinion!) Fair enough. CHANGES SINCE LAST ACTION https://reviews.llvm.o

[PATCH] D136154: [clang-format] Fix the continuation indenter

hel-ableton added a comment. I'm not sure I'm following where you're getting at. So far I'm getting the following: - my proposed fix was not ideal, and only "accidentally" fixed our issue - the fix including `Previous.isOneOf(TT_BinaryOperator...` is a better fix - we should write a proper test

[PATCH] D136154: [clang-format] Fix the continuation indenter

hel-ableton marked an inline comment as done. hel-ableton added a comment. > After you write a New Inline Comment, click the Save Draft button. Then click > the Submit button near the bottom of the screen. Makes sense, only I see no "Save Draft" button... Comment at: clang/li

[PATCH] D136154: [clang-format] Fix the continuation indenter

hel-ableton marked an inline comment as done. hel-ableton added a comment. I filed an issue now in the llvm repo, it just describes how the formatting in our case changes, and what _might_ be done to keep it from doing so. I have consciously left out any proposed unit tests, because at this poin

[PATCH] D136154: [clang-format] Fix the continuation indenter

hel-ableton added a comment. Hi Owen, thanks for reaching out, no. In fact we realized that we don't have enough capacity to be working on it. Best, Henrik - Henrik Lafrenz, Ableton AG Schoenhauser Allee 6-7, 10119 Berlin, Germany T: +49 30 288 763-256 Board: Gerhard Behles, Jan Bohl Supervis