Fork me on GitHub
#rewrite-clj
<
2020-12-22
>
dominicm21:12:02

Dumb request: Should rewrite-cljc move to line/col to mirror Clojure's own metadata rather than row/col?

lread21:12:18

Welcome to our little rewrite-clj world Dominic!

borkdude21:12:11

@lee I'm pretty sure he means the metadata keys :row and :col

borkdude21:12:42

I wish I had done this in edamame as well

borkdude21:12:51

it's now configurable

dominicm21:12:52

I'm not using the zip api, so I mean for this: (node.proto/form-meta (rewrite.parser/parse-string "(assoc {} :k 10)"))

dominicm21:12:17

For me, zip api is not useful I'm going to just go through the node api.

borkdude21:12:39

what does form-meta do apart from meta ?

borkdude21:12:53

I've never used that

dominicm22:12:32

No idea, I'm sticking to the node.protocols api though so I don't hit uncharted territory.

dominicm22:12:40

Not doing :tag, doing node.proto/tag, etc.

lread22:12:02

@borkdude it is new and might be going away… It was part of support for eliding positional metadata from readers. I might just hardcode the eliding instead. Breaking change but should not affect anyone.

borkdude22:12:25

> This metadata is not in the original source, and therefore assumed to be uninteresting to users of rewrite-cljc. Huh, the metadata is one of the critical parts I started using rewrite-clj in the first place ;)

lread22:12:14

It’s a metadata that comes from evaling an sexpr.

borkdude22:12:18

But I see the point of wanting to elide the location metadata when writing forms including metadata

borkdude22:12:15

Maybe sexpr should elide the metadata that wasn't on the form in the first place.

borkdude22:12:20

if it doesn't do that already

lread22:12:22

So @dominicm, some things might not make perfect sense, but if they have been around in rewrite-clj for a long while, we won’t change them in rewrite-cljc to avoid breaking compat with rewrite-clj.

lread22:12:54

Yeah @borkdude, I hit the problem in one place in Clojure, for quoted list:

(meta '(1 2 3))
{:line 1, :column 8}
And in many more with sci (which I think you might have since addressed).

lread22:12:22

I can’t even remember where I put eliding of metadata in rewrite-cljc, but have it on my todo to review.

lread22:12:41

This problem shows itself in rewrite-cljc on coercion from Clojure forrms to rewrite-cljc nodes. Which happens automatically in many places. So if I wanted to add a node via '(1 2 3) this would get coerced to to a rewrite-cljc metadata node without the eliding.

lread22:12:52

Which is probably never what the caller would want.

lread22:12:06

@dominicm that said, if you find stuff that is really irksome we might consider supporting through options, or in more extreme cases, breaking changes.

dominicm22:12:11

Include both maybe?

dominicm22:12:20

(If it's a worthwhile change, anyway)

dominicm22:12:30

it's very minor and bikesheddable, so :)

lread22:12:52

My todo list is long, but will listen if it is really bothering folks after the first official alpha release.

dominicm22:12:35

Is there anyway to know that a token node is a symbol, rather than ##Inf or something else special? (not sure what token nodes pick up)

borkdude22:12:03

@dominicm I have some of these helper functions in clj-kondo's code:

(defn symbol-token? [node]
  (symbol? (:value node)))

dominicm22:12:42

@borkdude But that's not part of the protocol! :P I suppose it can be specific to the token node interface though, yeah

borkdude22:12:49

In general it's best to avoid sexpr when you can, since that can blow up

borkdude22:12:09

so (symbol? (sexpr node)) isn't preferred

dominicm22:12:21

Seems like something that should be removed from the api? :P

borkdude22:12:37

Well, in some cases it's pretty darn convenient

borkdude22:12:09

but if you try to sexpr a map node with an uneven number of children, you're asking for it

lread22:12:33

There are nuances but if you know what you are targeting with sexpr it is handy.

borkdude22:12:40

also it's less performant to sexpr composite things when you can just look at some key

borkdude22:12:45

(defn boolean-token? [node]
  (boolean? (:value node)))

(defn char-token? [node]
  (char? (:value node)))

(defn string-from-token [node]
  (when-let [lines (:lines node)]
    (str/join "\n" lines)))

(defn number-token? [node]
  (number? (:value node)))

(defn symbol-token? [node]
  (symbol? (:value node)))

(defn symbol-from-token [node]
  (when-let [?sym (:value node)]
    (when (symbol? ?sym)
      ?sym)))

lread22:12:54

I really should add those… on my todo.

lread23:12:20

Oh, the -from- ones I hadn’t noticed…

dominicm23:12:35

For what I'm doing, knowing truer tag would be handy

lread23:12:19

Yeah tag is not very fine-grained is it?

borkdude23:12:30

the :token tag isn't very informative. one option is to add an extra field but this is one of those things that should have been in it since the beginning maybe

borkdude23:12:53

anyway, a few home-made predicates go a long way

lread23:12:24

yeah, I think adding the predicates is the way to go. I have added a node-type on my branch, but it is internal and to support testing. And it doesn’t answer the abstract question that a caller would be asking.

borkdude23:12:51

a function that returned some keyword could also be handy if you want to dispatch on something:

(defn tag+ [node] (if (symbol? (:value node)) :symbol) ...)
(case (tag+ node) :symbol ...)

borkdude23:12:19

I think I have something like that too in clj-kondo

borkdude23:12:00

this could also go into some extra utils lib

borkdude23:12:12

or utils ns

lread23:12:37

Hmm… interesting. If you have ideas and an interest, feel free to add them to: https://github.com/lread/rewrite-cljc-playground/issues/5

borkdude23:12:00

Just copy paste this conversation :)

borkdude23:12:36

I think it's already in there

lread23:12:48

ya.. can do.