Clojurians Log v2

wrote an issue about how leiningen merges maps - can’t remove values from those. Does anyone know/remember if there was a reason for this? or just a bug? https://github.com/technomancy/leiningen/issues/2556

@ikitommi I think I’ve seen nested meta supporting structures used instead for those cases before so the replace metadata worked.

But yeah, offhand I don’t know why the behavior is that way regarding nil. I guess sort of makes sense if that’s the merge semantic you were going for in some situations. Merging as combining instead of overwriting.

And lein mostly does combining merges of values

I think it would be a small change to check if the key exists and value is nil, and it should override the value to nil. Like it works with non-collections already. There are at least three copies of meta-merge, would not want to make a fourth copy with just this change.

Using it in all projects, really like the way it allows the user be in charge how things are merged.

@ikitommi I agree it’d be nice to do what you want in some cases. I don’t think it auto handling nil that way makes sense though.

I think there are cases where you’d not want nil to wipe out other values. Instead “combine” them

Maybe when the merging values are non-collections it may be ok. But that may be a weird stipulation.

I often wish meta merging let you use a placeholder value to wrap a value when you needed to add metadata like :replace to something that doesn’t support it

Like some predefined box type record

{:a ^:replace #MergeWrapper {:value nil}}

My use case is with the #reitit routing library. I use meta-merge to accumulate route data from a route tree into endpoints. It works like a charm in all cases but in the one where I one want’s to set some default in the top-level and override (remove) that default somewhere along the way. The common case:

(def router
  (r/router
    ["/api" {:interceptors [::api]}
     ["/ping" ::ping]
     ["/admin" {:roles #{:admin}}
      ["/users" ::users]
      ["/db" {:interceptors [::db]
              :roles ^:replace #{:db-admin}}]]]))

(r/routes router)
; [["/api/ping" {:interceptors [::api]
;                :name :user/ping}]
;  ["/api/admin/users" {:interceptors [::api]
;                       :roles #{:admin}
;                       :name ::users} nil]
;  ["/api/admin/db" {:interceptors [::api ::db]
;                    :roles #{:db-admin}}]]

but adding a content-negotiation is done my setting a :muuntaja key with a instance (non-collection) as a value, preferaby to the root, effecting all routes. With the current meta-merge impl, it’s impossible to unset the value without some marker values like false etc. The thing that meta-merge handles explicit nil differently than the clojure.core/merge makes me think it’s a bug.

If this would be fixed in Leiningen, the change would be mirrored to the meta-merge library I’m using (https://github.com/weavejester/meta-merge). Another option is to make a friendly copy and name it somethin like ctrl-merge. There is already a bit-different duct-meta-merge around (https://github.com/duct-framework/core/blob/master/src/duct/core/merge.clj).

@ikitommi yeah, I understand you have a usage

but I’m saying the :replace metadata already exists for the concept of “replacing”/overwriting

the downfall to it is that it only works on things that support metadata - like not on nil right?

yes, doesn’t work with nil.

So what I was saying

Was it’d be nice if the meta-merge transparently supported a “special wrapper type” that could take metadata for things that cannot have metadata

Then the same :replace mechanism could be used in any case - as well as :displace and whatever else

instead of hardcoding how something like nil is handled

hmm. I think the fix would be generic if it checked the map entry - if that exists and the value is not a collection, it would override.

Or maybe it’s all just about nil

And in that case, maybe top-level map could have metadata idk

as it works already with all non-collections, like (meta-merge {:a 1} {:a 2}) ; => {:a 2})

Yeah, it’s possibly all about nil

but you want nil to override, not sure all cases would want that - maybe you could justify as that’s just what non-collections do

I do wonder if things like lein take advantage of the current nil behavior though

There probably could be subtle issues changing it.

I was trying to find an example of that usage, but couldn’t find any..

I think some cases may be hard to see

Profiles merging and having default keys added etc

yeah, tons of private repos etc.

I don’t think it’s that uncommon for people to put :x nil in a map instead of not adding :x at all

in clj I find, you often have to go out of your way to not put the key in the map - it’s uglier

so in those cases, if that’s automatically happening, and it gets in front of a meta-merge - with your change, it’ll start wiping out other values instead of being ignored

so, too risky to change as something could break?

What I mean here: > in clj I find, you often have to go out of your way to not put the key in the map - it’s uglier (assoc m :x <don't know if I have this>) is commonly done, when really (cond-> m (some? <don't know if I have this>) (assoc :x <don't know if I have this>)) would be more appropriate

> so, too risky to change as something could break? I don’t really know. I’m just thinking it’d have to be looked into.

ok, thanks for giving it a thought. I understand if that can’t be changed, kinda fundamental and could break something - but there is now the issue of this.

just another option, add meta-merge2 and users can migrate as needed

accretion, not breakage

@alexmiller Yeah, definitely not a fan of using the same name for breaking things. I’m onboard with that concept. I’m not sure how the separate projects are kept in sync or whatever though - meta-merge vs lein.

I think it was already mentioned to copy/rename > If this would be fixed in Leiningen, the change would be mirrored to the meta-merge library I’m using (https://github.com/weavejester/meta-merge). Another option is to make a friendly copy and name it somethin like ctrl-merge. So that’s probably the meta-merge2 concept.

yep

If Leiningen doesn't need the nil thing, them the version 2 should be issue of the meta-merge library, right?

If there should be a bug in the merge, effecting both meta-merge1 & 2, should the fix be applied in both? Any guidelines when to stop maintaining old versions (like soon: spec1)?

also, if a libraryX updates from using mm1 to mm2, should that lib also change the namespaces so that "no-one breaks"?

would be nice to have some written guidelines for preferred clojure accretion with libs, ping @alexmiller

There are not always simple answers to all of these questions. Rich’s Spec-ulation talk is still the best rendering of the problems

In general, turn breakage into accretion

Fixation should generally be considered non-breaking as a matter of definition

But sometimes that is subtle if users rely on the “broken” behavior