Skip to content
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

Demo/proposal: FormattingFiler #664

Closed
wants to merge 2 commits into from

Conversation

ZacSweers
Copy link
Collaborator

Inspired by GJF's implementation of the same name - this demos an implementation of a FormattingFiler that formats kotlin code with KtLint while writing. This could also be applicable to the other writeTo() overloads, but would require slightly different support.

I don't think this should be part of the first party KotlinPoet API, but it could be a useful extension artifact. This filer-only API alone may be more valid as an extension artifact on KtLint's project, but as mentioend above the implementation could be useful for other writeTo() overloads too.

Example usage:

val formattingFiler = filer.asFormatting()

fileSpec.writeTo(formattingFiler)

Possible other writeTo() overloads could be FileSpec.formattedWriteTo() extension functions, but would likely want to reuse the directory preparation logic.

Happy to also just put all this in a separate project and call it a day. Talked with @Egorand offline and wanted to give first refusal here :)

Inspired by GJF's implementation of the same name - this demos an implementation of a `FormattingFiler` that formats kotlin code with [KtLint](https://github.com/pinterest/ktlint) while writing. This could also be applicable to the other `writeTo()` overloads, but would require slightly different support.

I _don't_ think this should be part of the first party KotlinPoet API, but it could be a useful extension artifact. This filer-only API alone may be more valid as an extension artifact on KtLint's project, but as mentioend above the implementation could be useful for other `writeTo()` overloads too.

Example usage:

```kotlin
val formattingFiler = filer.asFormatting()

fileSpec.writeTo(formattingFiler)
```
@JakeWharton
Copy link
Member

For those who think kotlinc isn't slow enough?

@ZacSweers
Copy link
Collaborator Author

ZacSweers commented Apr 7, 2019

For those that generate models/etc in separate, heavily-cached non-gradle subprojects :)

@Egorand
Copy link
Collaborator

Egorand commented Apr 21, 2019

Agree that this shouldn't live in the main artifact, but we can definitely move this into a separate one. We do plan to improve Kotlinpoet's own formatting though.

@ZacSweers
Copy link
Collaborator Author

Do you want me to set up a separate artifact as part of this? Or discuss it further/separately first?

@ZacSweers
Copy link
Collaborator Author

I envision this to be more useful for consumers that are implementing possibly complex CodeBlocks, and not necessary as a replacement for KotlinPoet's own standard formatting for standard types :)

@Egorand
Copy link
Collaborator

Egorand commented Apr 21, 2019

Yep, let's set up a separate artifact.

@LouizFC
Copy link

LouizFC commented Apr 25, 2019

We do plan to improve Kotlinpoet's own formatting though.

@Egorand could you elaborate please? Do you plan to use Kotlin official formatting guide as a base or this is just a backlog idea that isn't put much thought into?

@Egorand
Copy link
Collaborator

Egorand commented Apr 25, 2019

@LouizFC KotlinPoet already produces reasonably well-formatted code, but there are some issues we're aware of, such as function and constructor signature not being formatted according to the official style guide. This issue specifically is a top priority and I'm hoping to address it soon, if there's anything else that doesn't work for you - please file bugs.

@swankjesse
Copy link
Member

I'd prefer to not fix our formatting bugs by introducing support for pluggable formatters. One thing we learned with Dagger 2 is that nobody wants to wait for a formatting step.

@ZacSweers
Copy link
Collaborator Author

Going to close this for now as I think the lack of any direct touch points with KotlinPoet means it probably should just be a separate project

@ZacSweers ZacSweers closed this Aug 22, 2019
@ZacSweers ZacSweers deleted the z/formattingfilerDemo branch August 22, 2019 01:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants