Refactor to allow invoking check and fix as library fns w/ paths

[cap10morgan]

I wanted to use this with babashka as a library to check some paths recursively in a git hook, but noticed that the library fns (that don't call e.g. System/exit) took Clojure code in strings. I wanted to be able to give them a paths arg like you can do from the CLI and have it use the existing code to recursively find and ingest all the files to check. And then I needed it to return a report detailing which files failed the check, etc.

I changed the CLI / tool code to call these lib fns.

This is a big change, I know. So please let me know if it needs to be rethought, implemented differently, or whatever.

I've started using it in a git hook and it works great (and fast!).

[weavejester]

Can you explain a little further? e.g. can you give an example of cljfmt.lib vs cljfmt.tool? I'm not entirely sure what you're trying to achieve, so I'd love to get a more complete use-case.

[cap10morgan]

Can you explain a little further? e.g. can you give an example of cljfmt.lib vs cljfmt.tool? I'm not entirely sure what you're trying to achieve, so I'd love to get a more complete use-case.

Sure. I have a git pre-commit hook that runs via a babashka task. It runs cljfmt check and clj-kondo lint on my repo's Clojure code by (ideally) passing in paths like src, test, etc. and letting those tools check those paths recursively.

Since I want these git hooks to be fast and self-contained (so I don't need to require everyone else on my team to also install cljfmt and clj-kondo separately), I want to use cljfmt and clj-kondo as libraries.

clj-kondo already has a nice library fn that takes my src, test, etc. paths as an arg and lints the Clojure source files in those dirs recursively and returns a report on what it found that I can then use to pass or fail the commit and report to the committer what they should fix.

cljfmt, on the other hand, currently has fns like cljfmt.core/reformat-string that accept Clojure source code in strings and return the correctly-formatted version of that code. But for my git pre-commit hook it would be ideal if there were a library fn (that didn't do things like calling System/exit on failure) that could take paths as an arg, recursively check the formatting of Clojure source files in those paths, and then return a report on what it found so I can pass / fail the commit and tell the user what they should do next on failure.

cljfmt already has code to do all the work of recursively finding files under a starting directory and ingest any that contain Clojure source code, so I wanted to just reuse that if possible rather than writing my own. But a check fn that takes dirs to check and returns a report on what it found didn't already exist, so that's what this PR adds. It also makes fix work similarly for consistency's sake, but I didn't go quite as far with the refactor there since I'm not using fix in my git hook. Let me know if you'd like me to do more there.

The existing cljfmt.tool/check-no-config fn doesn't work well for my use case because it only returns counts (not which files failed the check, which I need to be able to tell the user which files they need to fix) and it calls System/exit when any files aren't correctly formatted, which would exit out of my git hook before I have a chance to tell the user anything at all.

I hope this clarifies the use case and motivation for the PR, but let me know if not.

[cap10morgan]

In case it's helpful, here is the code where I'm invoking clj-kondo: https://github.com/fluree/server/blob/98a2e3637d0c0119f12b456c925a4eb537283834/bb/lib/clj_kondo.clj#L16

...and this branch of cljfmt: https://github.com/fluree/server/blob/98a2e3637d0c0119f12b456c925a4eb537283834/bb/lib/cljfmt.clj#L11

[weavejester]

Okay, I believe I understand now. Let me think about how best to implement this. I don't think cljfmt.lib is the right name for the namespace, though I'm also not sure what you would call it. It may also be that we want to split out the recursive directory searches instead.

[cap10morgan]

Okay, I believe I understand now. Let me think about how best to implement this. I don't think cljfmt.lib is the right name for the namespace, though I'm also not sure what you would call it. It may also be that we want to split out the recursive directory searches instead.

Yeah, it made more sense to use cljfmt.core (though I don't love that convention), but I didn't want to break any existing users. I don't love cljfmt.lib either but didn't come up with anything better. Let me know if you do.

[cap10morgan]

@weavejester Any further thoughts on what should change in here to get this (closer to) mergeable?

[weavejester]

Apologies for taking a while to get back to you on this. My thought is that a separate namespace doesn't seem like the way to go. Instead, what about passing a reporter function, similar to how clojure.test works. e.g.

(report message-type message)

We could also roll exit code handling into this, as arguably exit codes are a type of reporting.

I don't think we need to factor out trace because it's for verbose logging, rather than reporting, and is off by default.

[cap10morgan]

Apologies for taking a while to get back to you on this. My thought is that a separate namespace doesn't seem like the way to go. Instead, what about passing a reporter function, similar to how clojure.test works. e.g.

(report message-type message)

We could also roll exit code handling into this, as arguably exit codes are a type of reporting.

I don't think we need to factor out trace because it's for verbose logging, rather than reporting, and is off by default.

OK, I'll look into all that. Thanks!

[cap10morgan]

Alright I have that working in this branch: https://github.com/cap10morgan/cljfmt/tree/feature/reporting-fn

Would you rather I force-pushed that to this PR or opened a new one?

[weavejester]

I think it feels different enough to warrant a separate PR, if that's okay by you.

[cap10morgan]

Closing in favor of a different approach