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

KTlint reporter arguments #347

Closed
JustinTullgren opened this issue Feb 8, 2019 · 9 comments
Closed

KTlint reporter arguments #347

JustinTullgren opened this issue Feb 8, 2019 · 9 comments

Comments

@JustinTullgren
Copy link

Hi,

I was wondering if it is possible to specify the reporter for ktlint? The userData map doesn't seem to send cli args but just editor config args, though I could be misunderstanding.

I have tried a few variations:

kotlin {
        ktlint(Versions.KT_LINT)
            .userData(mapOf("color" to "", "--reporter" to "checkstyle"))
}
kotlin {
        ktlint(Versions.KT_LINT)
            .userData(mapOf("color" to "", "reporter" to "checkstyle"))
}

If this is supported I would appreciate a pointer on how to accomplish this. If it is not I would request a feature enhancement for it.

Thanks!

@nedtwigg
Copy link
Member

nedtwigg commented Feb 8, 2019

@JustinTullgren
Copy link
Author

Thanks @nedtwigg for the quick reply. I am traveling this week but if no one volunteers by the time I get back I will take a stab at it. Thanks

@nedtwigg
Copy link
Member

Closing due to inactivity, happy to reopen on request.

@Ahmed-Abdelmeged
Copy link

Ahmed-Abdelmeged commented Dec 22, 2020

@nedtwigg any updates here? I want to get the ktlint reports to pass it to other tool.

@nedtwigg
Copy link
Member

No changes. Happy to take a PR for this feature, but as of now no one has plans to work on this.

@Ahmed-Abdelmeged
Copy link

I never did a plugin dev before but I will try my best to make this PR. Thanks! @nedtwigg

@nedtwigg nedtwigg reopened this Dec 22, 2020
@Ahmed-Abdelmeged
Copy link

Hi, @nedtwigg after a lot of investigation. KtlintStep uses the KtLint.fromat() internally and access it via reflection. And most of Ktlint useful functionally like reporters are a CLI commands written here. https://github.com/pinterest/ktlint/blob/master/ktlint/src/main/kotlin/com/pinterest/ktlint/Main.kt. I tried to port them to Spotless but that was too much work and felt like writing Ktlint internals again but in Java!
From my understanding of how Spotless internally work (Correct me if I'm wrong) You can't just call the library CLI commands instead of access its classes and call it via reflection.
Any suggestions?

@nedtwigg
Copy link
Member

nedtwigg commented Jan 4, 2021

Spotless is just a Function<String, String>, and that's all (see CONTRIBUTING.md).

  • The fastest way to implement this is to pass a string to a JVM function, and get a string back
  • But we also have steps (e.g. clang and black) where we pass a string on stdin and get the result on stdout, which requires a spawning a whole new process for every single file
  • The npm steps (e.g. prettier) even start a node.js server and pass data over localhost

For a JVM tool like ktlint, you get a huge speed improvement by calling a JVM function over and over so that the JIT can warm up. If you spawn a new process for every file, it will be very slow. But you'll still get buildcache and up-to-date caching, so it's not insane.

So you can absolutely build a ktlint step which calls java -jar ktlint.jar --stdin and then pass the file content on stdin, and parse the output on stdout, and wrap that whole mess as a Function<String, String>. But if ktlint doesn't have the public APIs that you need, it would probably be easier to get ktlint to accept a PR which adds them. Presumably ktlint is useful in lots of IDE-like scenarios, do they really intend the only API to be the CLI? If they do, I would at least have a way to pass arbitrary streams to use as stdin and stdout. Then you don't have to spawn a new JVM for every file, and can instead call ktlint.main(argsarray) and reuse the streams (something like this).

If the existing ktlint() step is too restrictive, I'd be open to adding a ktlintCli() or something which has the tradeoff that it is slower, but has more features. But mostly I think it would be best to get ktlint to commit to a public API so that an ecosystem can exist around it.

@nedtwigg
Copy link
Member

nedtwigg commented Dec 5, 2021

Now that #1012 has merged, it should be easier to build this if anybody wants to contribute a PR.

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

No branches or pull requests

3 participants