Dumb request: Should rewrite-cljc move to line/col to mirror Clojure's own metadata rather than row/col?
Welcome to our little rewrite-clj world Dominic!
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
@lee I'm pretty sure he means the metadata keys :row
and :col
I wish I had done this in edamame as well
it's now configurable
I'm not using the zip api, so I mean for this: (node.proto/form-meta (rewrite.parser/parse-string "(assoc {} :k 10)"))
For me, zip api is not useful I'm going to just go through the node api.
what does form-meta do apart from meta
?
I've never used that
No idea, I'm sticking to the node.protocols api though so I don't hit uncharted territory.
Not doing :tag
, doing node.proto/tag
, etc.
@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.
> 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 ;)
It’s a metadata that comes from evaling an sexpr.
But I see the point of wanting to elide the location metadata when writing forms including metadata
Maybe sexpr
should elide the metadata that wasn't on the form in the first place.
if it doesn't do that already
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.
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).I can’t even remember where I put eliding of metadata in rewrite-cljc, but have it on my todo to review.
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.
Which is probably never what the caller would want.
@dominicm that said, if you find stuff that is really irksome we might consider supporting through options, or in more extreme cases, breaking changes.
Include both maybe?
(If it's a worthwhile change, anyway)
it's very minor and bikesheddable, so :)
My todo list is long, but will listen if it is really bothering folks after the first official alpha release.
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)
@dominicm I have some of these helper functions in clj-kondo's code:
(defn symbol-token? [node]
(symbol? (:value node)))
@borkdude But that's not part of the protocol! :P I suppose it can be specific to the token node interface though, yeah
In general it's best to avoid sexpr
when you can, since that can blow up
so (symbol? (sexpr node))
isn't preferred
Seems like something that should be removed from the api? :P
Well, in some cases it's pretty darn convenient
but if you try to sexpr
a map node with an uneven number of children, you're asking for it
There are nuances but if you know what you are targeting with sexpr
it is handy.
also it's less performant to sexpr composite things when you can just look at some key
(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)))
I really should add those… on my todo.
Oh, the -from-
ones I hadn’t noticed…
For what I'm doing, knowing truer tag would be handy
Yeah tag
is not very fine-grained is it?
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
anyway, a few home-made predicates go a long way
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.
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 ...)
I think I have something like that too in clj-kondo
this could also go into some extra utils lib
or utils ns
Hmm… interesting. If you have ideas and an interest, feel free to add them to: https://github.com/lread/rewrite-cljc-playground/issues/5
Just copy paste this conversation :)
I think it's already in there
ya.. can do.