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

Why autostyle ? #615

Closed
LifeIsStrange opened this issue Jun 18, 2020 · 11 comments
Closed

Why autostyle ? #615

LifeIsStrange opened this issue Jun 18, 2020 · 11 comments
Labels

Comments

@LifeIsStrange
Copy link

autostyle/autostyle#20
I would like to have your opinion on the matter.
I want to use Klint and Klint mention both spotless and autostyle.
Autostyle is moslty unmaintained and not widely used contrary to spotless.
Spotless has more commits but what are the strengths of autostyle ? Why does the fork exists in the first place and did you try to fixes the original issues in order to obscolete the need of the fork ?

If it is actually obscolete I guess https://github.com/pinterest/ktlint should no longer mention autostyle.

@nedtwigg
Copy link
Member

One of the great things about opensource is that people are always free to fork and start a new project if they are unhappy with the policies or pace of change in an existing open source project. Spotless is a long-lived project which puts a heavy emphasis on backwards-compatibility. @vlsi had lots of great ideas that were difficult to implement without breaking things for existing Spotless scripts, and he was frustrated with some of our development methodology (we're pretty strict about changelog hygiene, which means that contributors often need to fix merge-conflicts in changelogs).

So @vlsi forked Spotless as autostyle, and I believe made a wide array of changes and improvements. For example, I know that #496 is still unsupported in Spotless, but is supported in autostyle.

@vlsi, if you're reading this, we happen to be gearing up for our first breaking-change in a long time, see #600. Most users won't need to change anything, but it could be a great opportunity to resolve some of your objections. We've also improved our changelog tooling since then. We'd love to have the benefit of your help again :)

@LifeIsStrange
Copy link
Author

LifeIsStrange commented Jun 18, 2020

Great answer @nedtwigg !
Thx :}

Also I believe that autostyle did use kotlin for development while spotless stick to 100% Java. Will you consider using Kotlin as a complement to Java where applicable? Or do you want to stick to pure Java?
Honestly learning Kotlin has been for me full of joy, it really improve the quality of life and fun of the developper life and is IMHO both clearer and more expressive.

But do whatever you want of course and I thank you a lot for the hard work!

@nedtwigg
Copy link
Member

I think Kotlin is fantastic. But Spotless serves a wide audience - Kotlin, Groovy, Scala, C/C++, Typescript, CSS, etc. The benefit of the wide audience is that many hands make for light work, which allows us to have excellent core infrastructure. The downside is that the Scala people want it to be Scala, the Groovy people want it to be Groovy, etc.

The core infrastructure is and will be lowest-common-denominator Java. If you to merge some Kotlin code to implement a Kotlin feature, that's probably fine. But if it's just a few lines of Kotlin, then we might as well transcribe it to Java to keep the requirements simple. And if it's a ton of Kotlin, then maybe it belongs as a PR to a kotlin project, exposed as a method call. But we do have languages besides java in our codebase where necessary, and we're happy to add more if there's a compelling reason.

@vlsi
Copy link
Contributor

vlsi commented Jun 18, 2020

Ah, licensing. That was indeed hard to approach with Spotless, so making Spotless work with different file formats (e.g. XML which should have <?xml..?> before license) was hard.


we happen to be gearing up for our first breaking-change in a long time, see #600.

That is something I remember as well. I do not want to spend time on supporting Gradle 2.x. It is too old.

Just a sample: modern Gradle has native IncrementalChanges processing, so incremental processing was re-implemented in Autostyle from scratch.


Most users won't need to change anything, but it could be a great opportunity to resolve some of your objections

Even though I reworked Gradle API in Autostyle, I'm inclined to drop the API to the very minimum.
I think the main configuration should be done with .editorconfig files.

For instance: trimTrailingWhitespace should be taken from trim_trailing_whitespace. The same for insert_final_newline, import order, and so on.

@vlsi
Copy link
Contributor

vlsi commented Jun 18, 2020

If you to merge some Kotlin code to implement a Kotlin feature

Kotlin's multiline strings make test cases way more readable: https://github.com/autostyle/autostyle/blob/3edcb79a2ddd0c8e6a988d5b6d8fd3ee2f9a729a/plugin-gradle/src/test/kotlin/com/github/autostyle/gradle/DiffMessageFormatterTest.kt#L120

        assertTaskFailure(
            task,
            """
            testFile
              @@ -2,9 +2,9 @@
               u 1
               u 2
               u 3
              -⇥·leading·space␊
              -trailing·space⇥·␊

Frankly speaking, adjusting those "multiline" test outputs in Spotless a huge pain.
Of course, that is not a show-stopper, but it is extremely sad that the project that focuses on formatting does not have proper infrastructure for the creation and maintenance of multi-line blobs:

assertCheckFailure(spotless,
" 00.txt",
" @@ -1,2 +1,2 @@",
" -1\\r\\n",
" -2\\r\\n",
" +1\\n",
" +2\\n",
" 01.txt",
" @@ -1,2 +1,2 @@",
" -1\\r\\n",
" -2\\r\\n",
" +1\\n",
" +2\\n",
" 02.txt",
" @@ -1,2 +1,2 @@",

@nedtwigg
Copy link
Member

Multiline strings is a good point, happy to accept Kotlin for tests. I don't want an "every test is Kotlin now" change, but if a test you're working on is improved by Kotlin, by all means :)

Also happy to take a PR for improved license step that isn't constrained to headers only, that is definitely a limitation.

@LifeIsStrange
Copy link
Author

@nedtwigg while I believe you should still accept Kotlin for tests,
for the specific case of kotlin raw strings, Java 13 has gotten a similar feature (as a feature preview): https://openjdk.java.net/jeps/355

@vlsi
Copy link
Contributor

vlsi commented Jun 18, 2020

happy to accept Kotlin for tests

Ok.


Then: Gradle comes by default with Kotlin, and Kotlin DSL is one of the two officially supported ones.
That makes writing Gradle plugins in Kotlin especially nice as the plugin code becomes similar to the code used in Gradle scripts.


Then, the current Java code is full of Objects.requireNonNull which is annoying. It looks like an attempt to have a null-safe code, however, it creates a lot of noise. Null safety can be enforced with checkerframework, yet I still think it would be easier to gradually migrate to Kotlin.

@nedtwigg
Copy link
Member

I love the strict null-safety in Kotlin and Typescript, I agree Objects.requireNonNull is terrible, I think the checkerframework is cool and I would welcome a PR that adds it, although I agree that a Kotlin migration is a much easier way to get null-safety than checkerframework.

Everyone who knows Kotlin can also develop for Java. But not everyone who knows Scala, Groovy, plain-Java etc already has a Kotlin environment set up.

Spotless is not the way I want it for me personally, and that's not my goal. I've tried to make it so that the largest possible community of people can contribute to it, and that sometimes means sacrificing to the lowest-common-denominator. The balance I try to make is saying "yes, do what you want" to as many people as possible, while also saying "yes, this still works" and "yes, we've used a language that you already know" to as many people as possible. That's the reason why I ask that the Spotless code remains java and that's why I'm happy to say yes to Kotlin tests.

@LifeIsStrange
Copy link
Author

There is also the https://projectlombok.org/features/NonNull annotation that might be an improvement

@nedtwigg
Copy link
Member

If you submit a PR, feel free to use Objects.nonNull or not, feel free to use @NonNull or @Nullable or neither. I don't care, and I'm not going to spend time strictly enforcing one standard or another. Our contributors have many different native tongues. The whole point of Spotless is to spend less time bikeshedding.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants