rewrite-clj

https://github.com/clj-commons/rewrite-clj
dominicm 2020-12-22T21:54:02.265600Z

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

lread 2020-12-22T21:56:18.266100Z

Welcome to our little rewrite-clj world Dominic!

lread 2020-12-22T21:56:47.266800Z

Are you talking about with postional support enabled? https://github.com/lread/rewrite-cljc-playground/blob/master/doc/01-introduction.adoc#tracking-position-with-the-zip-api

borkdude 2020-12-22T21:57:11.267400Z

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

borkdude 2020-12-22T21:57:42.267700Z

I wish I had done this in edamame as well

borkdude 2020-12-22T21:57:51.267900Z

it's now configurable

dominicm 2020-12-22T21:57:52.268Z

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

dominicm 2020-12-22T21:58:17.268100Z

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

borkdude 2020-12-22T21:58:39.268500Z

what does form-meta do apart from meta ?

borkdude 2020-12-22T21:58:53.268800Z

I've never used that

dominicm 2020-12-22T22:00:32.270Z

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

dominicm 2020-12-22T22:00:40.270300Z

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

lread 2020-12-22T22:01:02.270900Z

@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.

borkdude 2020-12-22T22:01:25.271500Z

> 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 ;)

lread 2020-12-22T22:02:14.272400Z

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

borkdude 2020-12-22T22:02:18.272600Z

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

borkdude 2020-12-22T22:03:15.274200Z

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

borkdude 2020-12-22T22:03:20.274500Z

if it doesn't do that already

lread 2020-12-22T22:03:22.274600Z

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.

lread 2020-12-22T22:04:54.275600Z

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).

lread 2020-12-22T22:05:22.276300Z

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

lread 2020-12-22T22:07:41.278400Z

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.

lread 2020-12-22T22:09:52.279400Z

Which is probably never what the caller would want.

lread 2020-12-22T22:19:06.280700Z

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

dominicm 2020-12-22T22:21:11.280800Z

Include both maybe?

dominicm 2020-12-22T22:21:20.280900Z

(If it's a worthwhile change, anyway)

dominicm 2020-12-22T22:21:30.281Z

it's very minor and bikesheddable, so :)

lread 2020-12-22T22:23:52.282Z

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

dominicm 2020-12-22T22:48:35.282200Z

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)

borkdude 2020-12-22T22:51:03.282700Z

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

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

dominicm 2020-12-22T22:52:42.282800Z

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

borkdude 2020-12-22T22:53:49.283900Z

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

borkdude 2020-12-22T22:54:09.284300Z

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

dominicm 2020-12-22T22:54:21.284400Z

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

borkdude 2020-12-22T22:54:37.284900Z

Well, in some cases it's pretty darn convenient

borkdude 2020-12-22T22:55:09.285800Z

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

lread 2020-12-22T22:55:33.286200Z

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

borkdude 2020-12-22T22:56:40.287600Z

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

borkdude 2020-12-22T22:57:45.288300Z

(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)))

lread 2020-12-22T22:58:54.288900Z

I really should add those… on my todo.

lread 2020-12-22T23:00:20.289400Z

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

dominicm 2020-12-22T23:00:35.289500Z

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

lread 2020-12-22T23:01:19.290100Z

Yeah tag is not very fine-grained is it?

borkdude 2020-12-22T23:02:30.291100Z

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

borkdude 2020-12-22T23:02:53.291400Z

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

lread 2020-12-22T23:04:24.293600Z

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.

borkdude 2020-12-22T23:05:51.294800Z

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 ...)

borkdude 2020-12-22T23:06:19.295100Z

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

borkdude 2020-12-22T23:07:00.295900Z

this could also go into some extra utils lib

borkdude 2020-12-22T23:07:12.296100Z

or utils ns

lread 2020-12-22T23:07:37.296400Z

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

borkdude 2020-12-22T23:08:00.296800Z

Just copy paste this conversation :)

borkdude 2020-12-22T23:08:36.297Z

I think it's already in there

lread 2020-12-22T23:56:48.297200Z

ya.. can do.