[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

owenpan added inline comments. Comment at: clang/lib/Format/ContinuationIndenter.cpp:1338-1341 PreviousNonComment->isOneOf( -TT_AttributeRParen, TT_AttributeSquare, TT_FunctionAnnotationRParen, -TT_JavaAnnotation, TT_LeadingJavaAnnotation))) ||

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

jaredgrubb updated this revision to Diff 557542. jaredgrubb added a comment. Address review comments, and adjusting the patch to address other merges since this patch was started. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145262/new/ https://reviews.llvm.org/D145262 Files:

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

owenpan added a comment. In D145262#4651483 , @owenpan wrote: > This looks OK to me. Please rebase after > https://github.com/llvm/llvm-project/pull/67518 is merged. I have merged the PR. Commits cef9f40cd4fe

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

owenpan added a comment. This looks OK to me. Please rebase after https://github.com/llvm/llvm-project/pull/67518 is merged. In D145262#4650300 , @jaredgrubb wrote: > Hopefully this will satisfy and we can merge! I didn't notice the Phabricator >

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

jaredgrubb marked an inline comment as done. jaredgrubb added inline comments. Comment at: clang/unittests/Format/FormatTestObjC.cpp:1619 + // Reflow after first macro. + // FIXME: these should indent but don't. + verifyFormat("- (id)init ATTRIBUTE_MACRO(X)\n"

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

jaredgrubb updated this revision to Diff 557274. jaredgrubb added a comment. Hopefully this will satisfy and we can merge! I didn't notice the Phabricator timeline and I'm hoping we can finish this before Oct 1. Rebased. Removed extraneous doc comment not related to patch (as requested).

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

owenpan added inline comments. Comment at: clang/lib/Format/ContinuationIndenter.h:231 /// Used e.g. to break like: + /// \code /// functionCall(Parameter, otherCall( Unrelated. Comment at: clang/lib/Format/ContinuationIndenter.h:234

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

jaredgrubb updated this revision to Diff 540477. jaredgrubb added a comment. Rebased and added a line to Release Notes, no other changes. Have requested this to be merged if all builds are green (since this patch has been approved). CHANGES SINCE LAST ACTION

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

jaredgrubb updated this revision to Diff 521953. jaredgrubb added a comment. 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 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145262/new/

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

jaredgrubb marked an inline comment as done. jaredgrubb added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:5473 + if (Right.isOneOf(tok::kw___attribute, TT_AttributeMacro)) +return true; + HazardyKnusperkeks wrote: > jaredgrubb wrote: >

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:5473 + if (Right.isOneOf(tok::kw___attribute, TT_AttributeMacro)) +return true; + jaredgrubb wrote: > HazardyKnusperkeks wrote: > > Does changing this return value

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

jaredgrubb added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:5473 + if (Right.isOneOf(tok::kw___attribute, TT_AttributeMacro)) +return true; + HazardyKnusperkeks wrote: > Does changing this return value make no difference? In other

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:4514-4515 +// Apply this logic for parens that are not function attribute macros. +if ((Left.is(tok::r_paren) && Left.isNot(TT_AttributeParen)) && +

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

jaredgrubb updated this revision to Diff 520195. jaredgrubb added a comment. 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 :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145262/new/

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/ContinuationIndenter.h:231 /// Used e.g. to break like: + /// ``` /// functionCall(Parameter, otherCall( MyDeveloperDay wrote: > If you want this to look like code you need to use \code

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/ContinuationIndenter.h:231 /// Used e.g. to break like: + /// ``` /// functionCall(Parameter, otherCall( If you want this to look like code you need to use \code \endcode CHANGES SINCE

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

jaredgrubb added a comment. I have uploaded patch for all the points you called out. (Sorry for delay; I missed the suggestions earlier!) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145262/new/ https://reviews.llvm.org/D145262 ___

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

jaredgrubb updated this revision to Diff 516211. jaredgrubb marked 6 inline comments as done. jaredgrubb added a comment. Address review comments from @owenpan CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145262/new/ https://reviews.llvm.org/D145262 Files:

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

owenpan added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:4380 +// Apply this logic for parens that are not function attribute macros. +if ((Left.is(tok::r_paren) && !Left.is(TT_AttributeParen)) && +canBeObjCSelectorComponent(Right)) {

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

jaredgrubb added inline comments. Comment at: clang/unittests/Format/FormatTestObjC.cpp:1619 + // Reflow after first macro. + // FIXME: these should indent but don't. + verifyFormat("- (id)init ATTRIBUTE_MACRO(X)\n" I don't love this FIXME, but I was afraid

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

jaredgrubb added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:5329 + if (Right.is(tok::l_square) && Right.is(TT_AttributeSquare)) return !Left.is(TT_AttributeSquare); I broke this `if` into two pieces and restored an original `return

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

jaredgrubb updated this revision to Diff 504486. jaredgrubb added a comment. - 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

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

HazardyKnusperkeks added inline comments. Comment at: clang/unittests/Format/FormatTestObjC.cpp:1528 + // This is added to acknowledge the behavior, but it should be improved. + verifyFormat("ATTRIBUTE_MACRO ATTRIBUTE_MACRO(X)\n" + "@interface Foo\n"

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

jaredgrubb updated this revision to Diff 502390. jaredgrubb added a comment. Create unit-tests for the patch (and remove the proposed non-unit test "test"). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145262/new/ https://reviews.llvm.org/D145262 Files:

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

owenpan added a comment. In D145262#4168058 , @jaredgrubb wrote: > 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

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

jaredgrubb added a comment. 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. If I do that, would that be in-addition-to or instead-of the test-case I

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

jaredgrubb added a comment. For background, the current clang-format results in the following (`-style="{BasedOnStyle: LLVM, ColumnLimit: 0, AttributeMacros: [MACRO]}`): MACRO MACRO(A) @interface Foo @end MACRO(A) MACRO @interface Foo @end This patch improves it

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

HazardyKnusperkeks added reviewers: rymiel, owenpan, MyDeveloperDay. HazardyKnusperkeks added a comment. Our tests reside in `clang/unittests/Format`, you want to look into the `TokenAnnotatorTests`. In fact **I** can't read the tests you wrote there. And could you please provide an example

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

jaredgrubb created this revision. jaredgrubb added reviewers: HazardyKnusperkeks, djasper, egorzhdan. jaredgrubb added a project: clang-format. Herald added a project: All. jaredgrubb requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. I