borkdude / edamame

Configurable EDN/Clojure parser with location metadata
Eclipse Public License 1.0
174 stars 14 forks source link

No location metadata on :read-cond :preserve items #47

Closed kkinnear closed 4 years ago

kkinnear commented 4 years ago

version

[borkdude/edamame "0.0.11-alpha.9"]

platform

OS/X 10.14.5

problem

When I specify :read-cond :preserve, the reader-conditional ends up in the output of parse-string-all, but does not have any location metadata. If I specify :read-cond :allow, and also specify the proper :features, I get the full location metadata on the resulting output from parse-string-all. The problem is, I don't necessarily know the correct :features to specify.

repro

(def ex "#?(:bb (defn zpmap [] (list :this :is :a :test)))")
(def expsa (edamame.core/parse-string-all ex {:read-cond :preserve}))
(meta (first expsa))
;=> nil
(def expsa (edamame.core/parse-string-all ex {:read-cond :allow :features #{:bb}}))
(meta (first expsa))
;=> {:row 1, :col 1, :end-row 1, :end-col 50}

expected behavior

Some background. I'm using edamame as a lightweight parser to scan a whole file to find the top-level expressions in the file. I am using only the location metadata from the results of parse-string-all. This is fast and works great for me -- except for one thing. If I specify :read-cond :preserve, then I don't get metadata on the top-level reader-conditionals. If instead, I specify :read-cond :allow then I get the correct location metadata on the output -- but only if I specify at least one feature that appears in the reader conditional. The problem is -- what if I didn't guess the correct features?

One approach would be to parse the file twice, once with :read-cond :allow and features, and once with :read-cond :preserve, and see if I get different numbers of things. If I did, I could find the differences and dig through the reader conditional and find the correct features and parse it again. But that is pretty ugly and slow as well.

What I'm hoping is that it wouldn't be a terrible thing to just return the location metadata on reader conditionals that were passed through with :read-cond :preserve:

(def ex "#?(:bb (defn zpmap [] (list :this :is :a :test)))")
(def expsa (edamame.core/parse-string-all ex {:read-cond :preserve}))
(meta (first expsa))
;=> {:row 1, :col 1, :end-row 1, :end-col 50}
borkdude commented 4 years ago

@kkinnear The items returned by {:read-cond :preserve} are of type clojure.lang.ReaderConditional and unfortunately these items can't carry metadata since they do not implement clojure.lang.IObj.

user=> (type (edamame/parse-string "#?(:clj 1 :cljs 2)" {:read-cond :preserve}))
clojure.lang.ReaderConditional
borkdude commented 4 years ago

An alternative would be to support a function: {:read-cond (fn [expr] ...)} so you can do your own thing with the parsed expression, would that help?

kkinnear commented 4 years ago

Thank you so much for getting back to me! The edamame parser is really a nice piece of work, and quite fast!

The function might be a help, I'm not sure. All I want is some way to unconditionally get something to show up in the output from parse-string-all that has the metadata on it. If you gave me a function with the expression as input, what would you do with the output? If I just always returned something like an empty list, would it get the metadata for where the expression was? That would work fine.

I may not be understanding what you are suggesting here, but maybe if the expr the function was given was a clojure.lang.ReaderConditional and I just wrapped it in a list or vector and returned that, then it could have metadata? That sounds better, and maybe more what you were thinking?

Alternatively, if there were some alternative value for :features that would cause it to ensure that some (any) of the features actually triggered so that something would come out with metadata, that would be fine too. Maybe :features :any? Kind of a hack, though.

Thinking more about this, I don't really understand what you would do with the results of the function that you propose. If you could help me understand that, I think I could answer more effectively.

Thanks again for this great parser!

borkdude commented 4 years ago

@kkinnear If the function would be useful, totally depends on your problem. It would receive the entire expression (:bb 0 :clj 1 :cljs 2) with location metadata {:row 20, :col 1} for example . We could also add to the metadata if the reader conditional was using splicing.

borkdude commented 4 years ago

So for example:

(edamame/parse-string "#?@(:bb 1 :cljs 2)" {:read-cond (fn [expr] (:edamame/read-cond-splicing (meta expr))}) ;;=> true
kkinnear commented 4 years ago

I think this would work out great for me. If I was using parse-string-all, I presume (hope?) that whatever the (fn [expr] ...) returns would go into the seq that is returned from parse-string-all? So, presumably, I could do something like this:

(def ex "#?(:bb (defn zpmap [] (list :this :is :a :test)))")
(def expsa (edamame.core/parse-string-all ex {:read-cond (fn [expr] (with-meta [] (meta expr)))}))
(meta (first expsa))
;=> {:row 1, :col 1, :end-row 1, :end-col 50}

That would be wonderful (and possibly useful to others as well, I hope).

borkdude commented 4 years ago

Implemented the above. Released as 0.0.11-alpha.10.

kkinnear commented 4 years ago

Thank you so much! This does exactly what I need. I really appreciate the quick turnaround. I wish I could respond so quickly to the requests I get! Thanks again for a great parser and your help in extending it for my needs.