weavejester/cljfmt

Options don't merge correctly (version 11)

sirmspencer opened this issue · 5 comments

sirmspencer commented

merge-configs was replaced with merge, and now its not doing a deep merge on indents. This commit e5eabdc

I see it in lein-cljfmt. Defaults are being overridden by project.clj or cljfmt.edn. I suspect it affect all places in the code where merge-configs was replaced.

sirmspencer commented

Here is a generic deep merge if you don't want to keep up with changing structure.

sirmspencer commented

I see it with clj -Tcljfmt fix as well.

weavejester commented

Sorry, can you explain a little more of what you expect to happen, and what you observe happening? I'm unsure if this is actually a bug, or if you haven't noticed the section on breaking changes.

sirmspencer commented

Oh hmm, yeah some of it was the breaking change.

But it still applies to lein-cljfmt if the cljfmt.edn file is used. :indents were deep merged before. When merging the project.clj :cljfmt object and .cljfmt.edn objects all :extra-indents from cljfmt.edn get fully overwritten by project.clj

We want the :extra-indents to deep merge right? Or are you thinking that if there are "any" :extra-indents in project.clj that they overwrite "all" the :extra-indents from .cljfmt.edn.

FYI, the old merge-configs

(defn merge-configs
  "Merge two or more cljfmt configuration maps together."
  ([a b]
   (-> (merge a b)
       (assoc :indents (merge (:indents a {}) (:indents b)))))
  ([a b & more]
   (reduce merge-configs (merge-configs a b) more)))
sirmspencer commented

I don't actually have an opinion now. I only plan on using the cljfmt file anyway.