Diff Detail
- Repository
- rC Clang
Event Timeline
Please do not commit without review. It is ok, to write ping every 5-7 days if there is no comment from the reviewers.
OK, so this is not a real bug in the sense of non-working current features, it's more like a feature request.
As per https://clang.llvm.org/docs/ClangFormatStyleOptions.html#adding-additional-style-options, I (and I'm sure other reviewers) would like to understand better this use-case before it's added to clang-format.
@owenpan can I help you bring this back up to date and review, I think I saw another issue in the bugzilla requesting this.
I know the code owners like new option to have a style guide used by a large code base, but to me this seems a reasonable improvement
@MyDeveloperDay: Can you add a link to the bugzilla report?
I stopped pushing my solution because as @krasimir said it's probably not a bug although the current documentation doesn't spell it out. Nonetheless, I'm with you and would like to see this improvement accepted.
Apart from the typo I think this is a simple enough change and a widely enough used style that it LG.
lib/Format/UnwrappedLineParser.cpp | ||
---|---|---|
181 ↗ | (On Diff #167225) | Typo: WrapBrace? |
include/clang/Format/Format.h | ||
---|---|---|
638 ↗ | (On Diff #167225) | This comment seems outdated (the one in ClangFormatStyleOptions.rst seems more recent). The ClangFormatStyleOptions.rst file is automatically generated from this file. Please update this comment and regenerate ClangFormatStyleOptions.rst by using the clang/docs/tools/dump_format_style.py script. |
lib/Format/Format.cpp | ||
611 ↗ | (On Diff #167225) | If Allman and GNU styles both follow the style suggested by this new flag, my mistake: this would be enough to add it to clang-format. |
@ownenpan might be worth checking with @reuk who has been adding some options for the JUCE style guide and looking at the JUCE code it seems this style might match their style.
include/clang/Format/Format.h | ||
---|---|---|
638 ↗ | (On Diff #167225) | @krasimir do you happen to know if this script is run by the build or is supposed to be run by the developer after making the change to Format.h If the latter, then I reckon the two are out of sync, perhaps I should submit a change to realign them, but really there should be some sort of make target that lets us determine when they diverge otherwise they'll keep changing |
Thank you to all for reviewing this revision! Here is the update that addresses all of your comments.
(Also added @reuk to the reviewer list per @MyDeveloperDay 's suggestion.)
do you happen to know if this script is run by the build or is supposed to be run by the developer after making the change to Format.h
I believe the developer must run it manually.
If the latter, then I reckon the two are out of sync, perhaps I should submit a change to realign them, but really there should be some sort of make target that lets us determine when they diverge otherwise they'll keep changing
+1