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

Custom FormatterStep with native dependencies #475

Closed
carbotaniuman opened this issue Oct 17, 2019 · 4 comments
Closed

Custom FormatterStep with native dependencies #475

carbotaniuman opened this issue Oct 17, 2019 · 4 comments

Comments

@carbotaniuman
Copy link

I'm looking to add a Clang-Format formatter step, but it requires a native exe file, which nothing else seems to need. Is there any way I can get Spotless to interface with a native formatter?

@nedtwigg
Copy link
Member

This is definitely possible. You can see it discussed in some detail here:

The only hard limitation imposed by spotless is Function<String, String>. The fast way to do this is to run formatter.exe -someflags, pipe the input to stdin, and read the result from stdout. The slower way is to write a temp file, run the exe, then read the result.

@carbotaniuman
Copy link
Author

carbotaniuman commented Oct 17, 2019

I can deal with the Function issue. It looks like #119 was about not having to shell out to the system for every call. I still don't know how you guys want native dependencies packaged (and for how many OSes), but do you anticipate shelling out to the system to adversely degrade performance (if clang-format can do stdio without files)?

@nedtwigg
Copy link
Member

how you guys want native dependencies packaged

One option is to find it on the system. Search the path or ask the user to specify explicitly, both are fine. Another option is for Spotless to download and extract the executable itself, which I'm also fine with.

However it is packaged, it's important that the executable have its FileSignature taken as part of the State for proper up-to-date checking, but that's a finishing-touch polish - doesn't matter to me if it's part of the initial PR or not.

do you anticipate shelling out to the system to adversely degrade performance

t(Function call) << t(stdio) << t(tempfiles). In any case, Spotless will bring incremental up-to-date, so only changed files will be checked. That makes performance of the formatters much less important. In #119, I think JIT-warmup of node.js was probably a big deal, which shouldn't be a problem for clang-format.

I'm sure that shelling to clang-format will be workable at worst, and probably it will feel plenty fast-enough. If it turns out to be slow, we can optimize it later, either by adding a "batch-file-call" functionality to Spotless, or by adding "daemon-mode" to clang-format.

@nedtwigg nedtwigg pinned this issue Jun 29, 2020
@nedtwigg nedtwigg unpinned this issue Jun 29, 2020
@nedtwigg nedtwigg pinned this issue Jul 1, 2020
@nedtwigg nedtwigg mentioned this issue Jul 2, 2020
@nedtwigg
Copy link
Member

An excellent suggestion @carbotaniuman! I implemented infrastructure for this in #672, and used clang-format and python's black as the motivating examples. Shipped in plugin-gradle 5.2.0 (tweet). The overall clang-format experience could be improved by #673...

@nedtwigg nedtwigg unpinned this issue Aug 25, 2020
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

2 participants