weavejester / cljfmt

A tool for formatting Clojure code
Eclipse Public License 1.0
1.11k stars 120 forks source link

Fix load-config for edn files with :file-pattern #308

Closed jean-lopes closed 1 year ago

jean-lopes commented 1 year ago

When trying to modify the :file-pattern via .edn config file we have one of the following scenarios:

cljfmt.edn with regex pattern

{:file-pattern #"very-weird-pattern"}

If we try to load this file, an exception is thrown:

Execution error at cljfmt.config/read-config (config.clj:33). No dispatch macro for: "

cljfmt.edn with a string

{:file-pattern "very-weird-pattern"}

The config is loaded, but when trying to apply the regex it fails because it is a String and not a regex pattern.

Execution error (ClassCastException) at cljfmt.tool/grep$fn (tool.clj:21). class java.lang.String cannot be cast to class java.util.regex.Pattern (java.lang.String and java.util.regex.Pattern are in module java.base of loader 'bootstrap')

I run into this problem on v0.10.5

weavejester commented 1 year ago

Thanks for the patch, but I'd rather do this via a #re tag.

jean-lopes commented 1 year ago

Changed the implementation, is this what you had in mind?

weavejester commented 1 year ago

Yes, that's what I was thinking of. I need to consider if that's the best solution though, and to add in some configuration tests.

In the meantime, you can of course use .cljfmt.clj instead.

jean-lopes commented 1 year ago

let me know if you want me to add said tests.

weavejester commented 1 year ago

So I've had a think about this, and I think it probably is the best solution, and would maintain compatibility with clojure-lsp. We probably want to add some tests, but I believe the config namespace is lacking tests anyway, so I'll go ahead and add some in so there's something to build off.