cortinico / ktfmt-gradle

A Gradle plugin to apply ktfmt to your builds, and reformat you Kotlin source code like a glimpse ๐Ÿงน๐Ÿ˜
MIT License
160 stars 20 forks source link

Add support for creating tasks to format script files (#337) #382

Open Roggstars opened 1 week ago

Roggstars commented 1 week ago

๐Ÿš€ Description

Following #337, I have added two tasks on the project level: ktfmtCheckScript and ktfmtFormatScript. These tasks will evaluate the formatting of top-level script (*.kts) files, like build.gradle.kts.

๐Ÿ“„ Motivation and Context

Currently, script files are ignored by the plugin. Having a consistent formatting between those and source files would be great.

๐Ÿงช How Has This Been Tested?

I have added (and adjusted) some integration and unit tests. The old tests actually had some formatting issues in the script files that were used to set up the project in a temp directory. I have separated those changes into separate commits for better legibility during review. I will happily squash them into the primary feature commit, if necessary and desired by you.

I have also published the plugin to my local maven and applied it to

Limitations As documented in the README, ktfmt is not supposed to be applied to the top-level build.gradle.kts. Therefore, the top-level script files are currently not formatted. Is this a limitation of the plugin itself? Do we want to be able to apply the plugin on the top-level (might be useful now that we actually want to format files that are there).

๐Ÿ“ฆ Types of changes

โœ… Checklist

simonhauck commented 1 week ago

Thanks for this amazing PR, i will have definitely a look at it later.

There is still one test failing. Could you have a look @Roggstars ?

Roggstars commented 1 week ago

I'll check it out, thought I had run the preMerge task locally...

Roggstars commented 5 days ago

I am still not 100% sure why the Windows machine keeps complaining about that single test. It fails because the build.gradle.kts file the test creates and appends is malformatted. I suspect this might have to do with different line separators on the different operating systems, but I am not 100% sure yet. I will check this out on my Windows machine and push a fix.

Edit: also, sorry for my force-pushes. Wasn't aware of all the messages this would create on the original issue. Will continue working on a branch in my fork now and only push to main when resolved. :)

cortinico commented 5 days ago

Edit: also, sorry for my force-pushes. Wasn't aware of all the messages this would create on the original issue. Will continue working on a branch in my fork now and only push to main when resolved. :)

No problem at all ๐Ÿ‘ I'll mark this as draft and please change it back to ready-for-review when you're done

Roggstars commented 10 hours ago

@cortinico Addressed the failing tests :) Happy to get and address any review/feedback/comments from your end!

The Gradle Wrapper Validation seems to fail right now. Is that something that can be fixed by just re-running?