square / kotlinpoet

A Kotlin API for generating .kt source files.
https://square.github.io/kotlinpoet/
Apache License 2.0
3.9k stars 288 forks source link

Add way to set column limit #1020

Closed rnett closed 3 years ago

rnett commented 3 years ago

Currently the column limit is hardcoded to 100 characters. I'd like to be able to configure this, perhaps on FileSpec like indent.

ZacSweers commented 3 years ago

What's the use case other than personal preference?

rnett commented 3 years ago

I'd like the generated code to match the rest of the project.

JakeWharton commented 3 years ago

Have you considered running a standalone formatter over it? The IntelliJ formatter can be run from the command line as a post-processing step. It's unlikely that our choices of line wrapping or indentation or other things will match, and using the same formatter as the rest of your project will ensure it looks like human-written code.

ZacSweers commented 3 years ago

Yeah you should do what Jake said. Here's an example for a Java one that Dagger uses in its code gen to format JavaPoet code.

https://github.com/google/google-java-format/blob/master/core/src/main/java/com/google/googlejavaformat/java/filer/FormattingFiler.java

https://github.com/google/dagger/blob/6a6d78289bc9fa11b17019eb4fd847c723e9e8fa/java/dagger/internal/codegen/ProcessingEnvironmentModule.java#L61

I've written such a thing before for KotlinPoet in a work project, it's not too hard to replicate the above with a tool like KtLint.

lpireyn commented 2 years ago

Wrapping can cause expressions to be syntactically illegal, e.g.:

// Assume the following line is already indented, e.g. single-expression function
... = predicate(x) && predicate(y) // Bad luck: this is column 100
    && predicate(z) // Syntax error

I know about the · symbol but it's cumbersome. Allowing one to specify a larger column limit would be much more programmer-friendly, especially when the generated Kotlin code doesn't need to strictly follow the coding style.

Can this issue be reopened?

JakeWharton commented 2 years ago

Changing the column limit doesn't change that behavior. You're just adjusting when it happens. If you want to solve that problem then solve it properly.

OndraZizka commented 1 year ago

@JakeWharton , could you please reconsider this? Not having this configurable requires ugly workarounds for pretty standard code. Like:

                            throw %2M("Invalid value for field '${propertyName}', expected an array.")

This breaks because KotlinPoet breaks the string.

With the dots it's uglier:

                            throw %2M("Invalid${NBSP}value${NBSP}for${NBSP}field '${propertyName}',${NBSP}expected${NBSP}an${NBSP}array.")

With an additional string it's also uglier:

                            throw %2M("Invalid value for field '${propertyName}', " 
                                    + "expected an array.")

So. Could you please add the configurable line length? 100 is your personal preference, I understand. We have 160, and this generated source looks a lot out of the place.

Thanks.

OndraZizka commented 1 year ago

This is effectively a bug, since if someone is not in control of the length of the strings, it may fail randomly based on the length strings added to it.

OndraZizka commented 1 year ago

This is effectively a bug, since if someone is not in control of the length of the strings, it may fail randomly based on the length strings added to it.

Or what is the proper way to do that? I am reading the guide and see %L, but not sure how to apply it in this context.

JakeWharton commented 1 year ago

File a new issue about the bug. It should not be breaking in strings. We have tests for that behavior.