User Details
- User Since
- Jun 20 2017, 12:24 AM (310 w, 1 d)
Mon, May 22
Address review comments: unroll a loop in unit tests to explicitly test all the property attributes.
Sat, May 13
Address review comments:
- remove redundant &&
- remove part of patch that was not tested by any test and should be its own patch on its own merits
Address review comments:
- fix some style
- add unit test for each ObjC attribute recognized by the compiler
- adjust the docs for the style-option to show a YAML example with all of them in a sane order (something people could copy-paste as a starter version)
Sun, May 7
Update ClangFormatStyleOptions.rst as requested by the auto-comment (thanks auto-bot!).
Some implementation notes:
- The implementation was modeled after QualifierAlignmentFixer and sortCppIncludes.
- the additions to the .bazel/.gn build files was done naively based on searching for where "QualifierFixerTest" appeared in the repo; I can't really test these. It looks right, but it's a guess.
- I considered creating a top-level style similar to QualifierAlignment/QualifierOrder; that one has a few pre-canned values (Leave, Left, Right, Custom), where Custom opens up the style QualifierOrder, an array of qualifier names.
- Pro: we could create a [Leave, LLVM] that would set them in the same order the DeclPrinter.cpp would dump them
- Pro: users probably would like not having to think about 18 different attributes (I did try to document them in groups to make them easier to find)
- Con: I couldn't find any style guides that provide a "standard order", so I don't know any other pre-canned settings that wouldn't just be an opinion of my own personal style. If there are, let me know and that might add weight to going this route.
Address review feedback about code/endcode. Otherwise the patch is the same and I hope ready for merge? I'd love to get a green check :)
Apr 23 2023
I have uploaded patch for all the points you called out. (Sorry for delay; I missed the suggestions earlier!)
Address review comments from @owenpan
Mar 23 2023
Manuel, if you're happy with the change, do you mind committing it? I don't have commit access (at least I've never requested it, so I assume I can't, I've never tried)
Yeh, I considered trying to craft one as courtesy but this seemed like a very far edge case and didn't seem really worth it. So glad you agree :)
Mar 20 2023
Mar 17 2023
The difference doesn't appear to affect any unit tests (which is unfortunate), but I think you didn't mean to remove this else, based both on the logic of the original commit and the format of the patched line.
Mar 12 2023
- Fixed an issue in TokenAnnotator about it not breaking between macros properly (it was catching in an ObjC selector-check too early)
- Add more ObjC tests, covering method and property declarations too. There are still some quirks about reflowing multiple attributes, but those quirks exist in C++ too, so I think those are best left for another patch. I added checks for existing behavior so that patch can improve the ObjC version too.
Mar 4 2023
Create unit-tests for the patch (and remove the proposed non-unit test "test").
Mar 3 2023
I wasn't sure about testing (this is my first patch!) and the test-case I did was in clang/test/Format .. I can look at clang/unittests/Format and see how to model something like it.
For background, the current clang-format results in the following (-style="{BasedOnStyle: LLVM, ColumnLimit: 0, AttributeMacros: [MACRO]}):
MACRO MACRO(A) @interface Foo @end