- User Since
- Jul 10 2012, 10:15 AM (254 w, 4 d)
Lets try this the other way around. I am not ok with introducing an additional top-level option for this. It simply isn't important enough. So find a way for the existing style flags to support what you need and not regress existing users. If that can't be done, I am also ok with adding another value into BraceWrapping (which you suggested at some point, I think).
I think it's just wrong that WebKit inherits this. The style guide explicitly says that this is wrong:
I don't understand. WebKit style would not set AllowShortFunctionsOnASingleLine and so the behavior there wouldn't change, I presume?
In all honesty, I think this style isn't thought out well enough. It really is a special case for only "=" and "return" and even there, it has many cases where it simply doesn't make sense. And then you have cases like this:
Wed, May 24
I generally would not be opposed to such a patch. However, note that this might be hard to get right. We had significant performance problems in the past with ForEachMacros as we used to match every single identifier against the regex stored in there. For for loops you can somewhat get out of that and you might be able to do the same thing here, but I am not entirely sure. In contrast, the added value is actually not very large. clang-format is merely not able to automatically fix something to your liking and it's very easy to make the code right and have clang-format keep it that way.
No, I don't think it should be done this way and neither Facebook nor Mozilla coding styles say you should.
I don't. Only if they start out to be on the same line. As long as I start with:
clang-format already has logic to detect semicolon-less macro invocations an in fact this already does behave as I would expect. What are you fixing?
But that style specifically says that it is only done if the initializer list is wrapped:
Looks good. Thank you!
As it currently stands, I am really not happy with the configuration space that this opens up. If we can't make the configuration of existing flags, what's the coding style encourages this behavior?
Does anything speak against making this behavior happen with AllowShortFunctionsOnASingleLine = SFS_Empty and MergeEmptyOnly.BraceWrapping.AfterFunction = true? I mean without the extra style option?
Tue, May 23
Don't C99 designated literals use "=" instead of ":"?
That's what I meant by "The name NamespaceIndentation might then be a bit confusing, but not sure whether it's worth changing it.".
Mon, May 22
What I mean is that you should remove the CompactNamespace option and instead let this be configured by an additional enum value in NamespaceIndentation.
Hm, can't really remember what I meant by "my comment". Probably not important.
Fri, May 19
I think we should just not do this for now. This addresses a very infrequent case that's easy enough to fix manually. As such, it's not worth the added complexity of clang-format and potential failures it might generate. Changing non-whitespace/non-comment code is always dangerous.
Thu, May 18
Yes, this definitely does not belong in the NamespaceEndCommentsFixer. It has nothing to do with comments.
When you say "this doesn't happen in tests", do you mean this never happens when there are parentheses around the expression?
Wed, May 17
I have looked through the PDF you linked to, but I couldn't really find much about formatting. I have found one instance of a constructor initializer list, but there is no accompanying text saying that this is even intentional. Haven't found a range-based for loop. As such, I am not convinced that this option carries it's weight.
Yes, turning that option into an enum seems like the better choice here.
The current behavior here is actually intended. If you'd like the other format, we probably need to add a style option. What style guide are you basing this on?
Basically looks good, but please add a test case where the penalty is high show that it changes behavior.
Are there cases where this makes a difference? If so, add a test. If not, add something to the patch description to clarify.
Mon, May 15
A test case might be useful.
Sun, May 14
Fri, May 12
Probably all of the examples from the original patch description and later comments should be turned into unit tests.
Thu, May 11
Wed, May 10
One nit, otherwise looks good.
Tue, May 9
Please read and address: https://clang.llvm.org/docs/ClangFormatStyleOptions.html#adding-additional-style-options
Mon, May 8
Submitted as r302427.
Submitted as r302428.
Sat, May 6
Fri, May 5
Thu, May 4
Wed, May 3
Tue, May 2
This is an edge case in actual C++. An escaped newline literally gets expanded to nothing. So what this reads is
Mon, May 1
Apr 27 2017
Apr 26 2017
My point is though that even with only one argument, the BinPackArguments setting might lead to this bug. If not, that's good :).
What happens if the function call where this happens actually does not have multiple parameters but one parameter with many operands, e.g. changing your test case to:
Ping? Do you think this is useful?