New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for context-receivers #1233
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you open to implementing the code gen part of it? I don't think it would be too difficult.
General design looks ok, left comments where there need to be changes.
In the future, please don't PR unrelated formatting changes that deviate from the code style we use, it makes the PR quite noisy.
kotlinpoet/src/main/java/com/squareup/kotlinpoet/ContextReceivers.kt
Outdated
Show resolved
Hide resolved
kotlinpoet/src/main/java/com/squareup/kotlinpoet/LambdaTypeName.kt
Outdated
Show resolved
Hide resolved
kotlinpoet/src/main/java/com/squareup/kotlinpoet/LambdaTypeName.kt
Outdated
Show resolved
Hide resolved
I was going to propose an implementation with this design as well; however, I didn't I ran into an issue, which is why I just proposed a design, once that is finished I would be open to implement the code gen
I didn't mean to do that, looks like IntelliJ just ignored your |
I hope I caught all the code style violations? |
kotlinpoet/src/main/java/com/squareup/kotlinpoet/LambdaTypeName.kt
Outdated
Show resolved
Hide resolved
kotlinpoet/src/test/java/com/squareup/kotlinpoet/FunSpecTest.kt
Outdated
Show resolved
Hide resolved
- Use proper experimental annotation - Remove not needed overloads - Move contextReceivers parameter to the end
Co-authored-by: Zac Sweers <pandanomic@gmail.com>
- Add more tests - Add new-line after context()
…linpoet into feature/context-receivers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sweet! Added a few comments, some of them (non-nit ones) would be nice to address before merging.
kotlinpoet/src/main/java/com/squareup/kotlinpoet/ExperimentalKotlinPoetApi.kt
Outdated
Show resolved
Hide resolved
kotlinpoet/src/test/java/com/squareup/kotlinpoet/FunSpecTest.kt
Outdated
Show resolved
Hide resolved
kotlinpoet/src/main/java/com/squareup/kotlinpoet/LambdaTypeName.kt
Outdated
Show resolved
Hide resolved
kotlinpoet/src/test/java/com/squareup/kotlinpoet/LambdaTypeNameTest.kt
Outdated
Show resolved
Hide resolved
kotlinpoet/src/test/java/com/squareup/kotlinpoet/LambdaTypeNameTest.kt
Outdated
Show resolved
Hide resolved
@DRSchlaubi also please sign our CLA before we can merge you PR. |
Done |
Co-authored-by: Egor Andreevich <github@egorand.dev>
5631a95
to
1b1a5d9
Compare
Co-authored-by: Egor Andreevich <github@egorand.dev>
Think it just needs a |
kotlinpoet/src/main/java/com/squareup/kotlinpoet/ExperimentalKotlinPoetApi.kt
Outdated
Show resolved
Hide resolved
|
||
assertThat(funSpec.toString()).isEqualTo( | ||
""" | ||
|context(@com.squareup.kotlinpoet.FunSpecTest.TestAnnotation kotlin.String) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you confirm this is legal syntax?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using it in IntelliJ crashes intellij, however there is no real rules in the KEEP for how to use annotations and the compiler accepts it
kotlinpoet/src/test/java/com/squareup/kotlinpoet/FunSpecTest.kt
Outdated
Show resolved
Hide resolved
kotlinpoet/src/main/java/com/squareup/kotlinpoet/LambdaTypeName.kt
Outdated
Show resolved
Hide resolved
…e.kt Co-authored-by: Zac Sweers <pandanomic@gmail.com>
kotlinpoet/src/main/java/com/squareup/kotlinpoet/LambdaTypeName.kt
Outdated
Show resolved
Hide resolved
Co-authored-by: Egor Andreevich <github@egorand.dev>
Merged, thank you! |
This PR aims at implementing support for context receivers (#1232)
It currently only tries to implement a possible design for builders and specs and has no code generation logic
Fixes #1232