clojure-emacs / orchard

A fertile ground for Clojure tooling
Eclipse Public License 1.0
326 stars 54 forks source link

`info*`, offer `:var-meta-allowlist` option #228

Closed vemv closed 7 months ago

vemv commented 7 months ago

Part of https://github.com/clojure-emacs/cider-nrepl/issues/851

It allows consumers to specify var metadata beyond Orchard's fixed whitelist.

Cheers - V

vemv commented 7 months ago

@bbatsov PR ready, the master builds are red due to Clojure 1.12

(investigating this weekend)

vemv commented 7 months ago

The fix was actually easy - done.

bbatsov commented 7 months ago

These days I try to avoid the whitelist terminology for various reasons, so let's name this something else - e.g. var-meta-keys or something along those lines. (basically something reflecting better we are selecting only a subset of the var metadata - meaningful keys, useful keys, etc) Let's also add an alias to the old var matching whatever name we pick here.

vemv commented 7 months ago

Good catch! Will refactor

vemv commented 7 months ago

Ready

bbatsov commented 7 months ago

I meant using something more specific than a generic name like allowlist, but anyways - that's not a big deal. The conversation around blacklist vs allowlist also made me realize how bad such names are in general, as they don't carry much meaning. In our case we want only a certain properties because we don't care about the others and they might have content that we can't parse. That's not really an allowlist, but something more specific IMO, but I guess I didn't manage to convey this clearly.

vemv commented 7 months ago

Ah, I see, sorry about that. Could have been selected-keys (following select-keys).

Although there's some value in using a term that is well-understood, even if it doesn't always exactly map to what it means.