- User Since
- Jul 10 2012, 10:15 AM (327 w, 4 d)
Thu, Oct 18
Ok, I think I agree with both of you to a certain extent, but I also think this change as is, is not yet right.
Aug 15 2018
Aug 1 2018
I don't have very strong opinions here and I agree that we will need to improve the conditional formatting, especially as this kind of ternary-chaining is becoming more popular because of constexpr.
Jul 30 2018
Ok, so IIUC, understanding that @end effective ends a section much like "}" would address the currently observed problems?
Jul 23 2018
In my opinion, this only addresses one edge case where clang-format -lines output is not identical with a full reformatting. I believe they cannot all usefully be avoided. As such, I am unsure that this option carries its weight of making the implementation more complex.
How often does this happen for you? Why can't you just format the additional incorrect line?
Could you explain what problem this is fixing?
Jul 12 2018
Jun 29 2018
Jun 26 2018
Jun 12 2018
You are right that this behavior is what the code authors, but also many other people, like to have and so it is what is engrained in clang-format. There are likely about a million things that fall into the same category. Now we might find that the current default is actually wrong and we have changed many defaults in the past. I don't believe that's the case here and there are more opinions on this thread.
Jun 11 2018
Looks good. Sorry for the delay.
The normal rule for formatting options apply. If you can dig up a public style guide and a project of reasonable size where it is used, we can add an option.
Jun 5 2018
May 20 2018
May 8 2018
Generally looks good.
Apr 27 2018
Looks good, thank you.
Apr 20 2018
Apr 19 2018
Apr 12 2018
I understand that, but the test example does not break after the closing paren. It breaks after the subsequent "<".
Both patch and comment inside patch say "break after closing paren" and yet that's not what the test or example in the description actually do. Why is that?
Apr 6 2018
Ok, you know the ObjC community much better than I do. If you think that adding the indentation for ObjC irrespective of the option works for most people, I am happy to go with it.
Apr 5 2018
I'd go to great lengths to avoid adding new config options and so I don't think this would be a bad trade-off to make.
Looks good, thank you!
I still really believe that these config options do no make sense and are actively confusing.
Apr 3 2018
Apr 2 2018
Looks good. I like option 2 :).
Mar 30 2018
Mar 29 2018
Well, I disagree. It says: "If you break after the return type of a function declaration or definition, do not indent."
Do we have a public style guide that explicitly says to do this?
That's a requirement for new style options as per https://clang.llvm.org/docs/ClangFormatStyleOptions.html#adding-additional-style-options.
Mar 27 2018
Yeah, it's one of these things where neither way would be totally intuitive to everyone.
Generally looks good, one minor simplification.
Mar 26 2018
Mar 23 2018
Mar 22 2018
Looks good, thank you!
Some last comments, but basically looks good.
Generally looks good.
Mar 21 2018
Please generally upload diffs with the full file as context so that Phabricator can properly expand the context where necessary.
Mar 20 2018
I'd really like to not parse include/import statements this way. Can you send me examples of headers where we fail to recognize them as ObjC and this matters (happy for you to send me examples offline).
Mar 12 2018
Ok, looks good.
Mar 8 2018
Nice.. Removed a lot of complexity :). Let's see whether this heuristic is good enough.
Mar 7 2018
Mar 6 2018
Right. So the difference is that for blocks, there is always a "(" after the "(^.....)". That should be easy to parse, no?
Mar 5 2018
Would it be enough to only add the block type case? With the macro false positive, there won't be an open paren after the closing paren, right?
Mar 2 2018
We have talked about that and none of us agree.
Got two responses so far.
Mar 1 2018
Are you sure that you are even addressing an important case? I have done some research on our codebase and this is something that happens incredibly rarely. The reason is that you have to have a very specific combination of line length, where the last parameter does not fit on one line if indented back to align with the open paren while it does fit on multiple lines if indented right of the comma. In all of LLVM/Clang, there seem to be only about 50 cases. How certain are you that people actually care?
Feb 28 2018
Ah, I thought it was somehow possible to create:
New options for this would not be acceptable IMO. Too much cost for too little benefit.
If both this and https://reviews.llvm.org/D32525 are submitted, then we also need more tests for the combination of the two parameters.
I think this generally looks good, but needs a few more tests.
But you *do* want extra indentation in the case of:
I think it's possible that this is just a bug/oversight. But I don't fully understand the case where it is not behaving as you expect. Can you give me an example (config setting + code that's not formatted as you expect)?
Feb 27 2018
Feb 23 2018
Submitted as r326023.