- User Since
- Jul 10 2012, 10:15 AM (271 w, 5 d)
Thu, Sep 21
Tue, Sep 19
I think doing the computation twice is fine. Or at least, I'd need a test case where it actually shows substantial overhead before doing what you are doing here. Understand that creating more States and making the State object itself larger also has cost and that cost occurs in the combinatorial exploration of the solution space. Doing an additional computation at the end should be comparatively cheap. Look at it this way: During the exploration of the solution space, we might enter breakProtrudingToken many times for the same comment. One more time during reconstruction of the solution is not that harmful.
Mon, Sep 18
Thu, Sep 14
I still don't understand yet. breakProtrudingToken has basically two options:
Tue, Sep 12
I'd still prefer individual patches for each of these changes. If the code review system or VCS make it hard for you to deal with two adjacent changes this way, do them in sequence.
BraceWrapping.AfterExternC makes sense to me.
I have a slightly hard time grasping what this patch now actually does? Doesn't it simply try to decide whether or not to make a split locally be comparing the PenaltyBreakComment against the penalty for the access characters? If so, couldn't we simply do that as an implementation detail of breakProtrudingToken() without needing to let anything outside of it now and without introducing State.Reflow?
I am very strongly against a flag that just leaves the line break as is. What's the motivation?
Mon, Sep 11
Thu, Sep 7
Submitted as r312721. Thank you.
Looks good. Sorry for the delay.
Also run dump_format_style.py and keep the changed .rst file in this change.
This change needs to be made to include/clang/Format/Format.h and then the rst file needs to be regenerated with docs/tools/dump_format_style.py.
Wed, Sep 6
Note that these changes need to be made to the corresponding comments in include/clang/Format/Format.h and then this file is auto-generated with docs/tools/dump_format_style.py.
Tue, Sep 5
Mon, Sep 4
Sun, Sep 3
Fri, Sep 1
Thu, Aug 31
Wed, Aug 30
Tue, Aug 29
Just a few minor comments, otherwise looks good.
Are you changing the line endings here? Phabricator tells me that basically all the lines change. If so, please don't ;).
Looks good. Thank you.
Mon, Aug 28
Yay for *removing* complexity for a change :).
Let me know how it goes in practice.
Sun, Aug 27
Aug 25 2017
Aug 24 2017
From my side this looks good for now (we can always improve more later). Krasimir, what do you think?
Does the test still test the same thing if you set the column limit to 60 and remove 20 spaces? If not, this is fine.
Aug 23 2017
Aug 22 2017
Krasimir: Can you actually give this a round of review? I will also try to do so tomorrow.
If no tests break with this, lets just go for it. We can follow up and fix individual cases if we find undesired behavior.
Aug 21 2017
Aug 16 2017
Aug 14 2017
Aug 10 2017
Aug 9 2017
Looks good. Thank you!
Aug 8 2017
Manuel: Can you take a look at the last comment here? Why does PPBranchLevel start at -1?
Aug 6 2017
Aug 4 2017
Aug 3 2017
Aug 2 2017
Aug 1 2017
Generally, please upload patches with full context to phabricator. (or use arc)