Fork me on GitHub
#sci
<
2020-08-02
>
kwrooijen10:08:27

Hey, I'm working with Edamame at the moment and something I'm currently bumping into is that I don't know the exact position of non meta objects. It would be nice if I could get access to that data somehow. I was thinking maybe a flag to return a wrapper record for non meta types? e.g. somewhere in https://github.com/borkdude/edamame/blob/1357bca1cd38434394028e6280b60ba52a25d83d/src/edamame/impl/parser.cljc#L494-L500 With the following code added

(defrecord EdamameValue [value])
,,,

(with-meta (EdamameValue. obj)
  {(:row-key ctx) (:row loc)
   (:col-key ctx) (:col loc)
   (:end-row-key ctx) (:row end-loc)
   (:end-col-key ctx) (:col end-loc)})
Just a suggestion, not sure if it's a good solution. I would realy benefit from the extra information though. Thoughts?

kwrooijen10:08:32

I noticed that obj can also be :edamame.impl.parser/expected-delimiter which should be ignored in this case. I don't know if there are any other special forms like this

borkdude10:08:15

Can you give a concrete example of how you would use this kind of output? FWIW, I'm using rewrite-clj in clj-kondo to get access to all locations, it returns wrapped values.

borkdude10:08:25

I do think edamame could also return these wrapped values, but maybe a function to write your own wrappers might be even more flexible

borkdude10:08:41

as it already accepts functions and options for most stuff

kwrooijen10:08:57

For example right now I'm checking if a value is the correct type. And if it's not I want to output the row / col of that value.

kwrooijen10:08:48

(Basically writing a simple compiler)

kwrooijen11:08:31

Or transpiler I should say

borkdude11:08:37

right. so we could hook in a simple function here: https://github.com/borkdude/edamame/blob/1357bca1cd38434394028e6280b60ba52a25d83d/src/edamame/impl/parser.cljc#L494-L500 that gets access to the new obj and metadata, so you could hook in your own thing there

kwrooijen11:08:23

That would be great. Something like a multmethod that matches on type? Or what did you have in mind?

borkdude11:08:43

no, nothing like that, just a function (fn [{:keys [:object :location]}]) so you can hook in your own implementation of what you want there

borkdude11:08:03

let me hack together an example

borkdude11:08:19

first, lunch

kwrooijen11:08:39

No rush, thanks! 👍

kwrooijen11:08:16

Something like this then?

borkdude11:08:30

yeah, although I would use a map argument instead of 2 positional ones, so it can be extended

kwrooijen11:08:07

Cool, I'll make a PR then

borkdude11:08:45

no need, I'm already working on it

borkdude11:08:01

but maybe it would be fun to see if you would come up with the same 😉

kwrooijen11:08:22

Probably won't though 😛

borkdude11:08:17

@kevin.van.rooijen So this is what will happen there:

user=> (parse-string "[1]" {:obj-fn (fn [{:keys [:obj :loc]}] {:obj obj :loc loc})})
{:obj [{:obj 1, :loc {:row 1, :col 2, :end-row 1, :end-col 3}}], :loc {:row 1, :col 1, :end-row 1, :end-col 4}}

borkdude11:08:42

Pushed a commit to a branch: f1f6cfb1f3c2e3057902fc8b92a05c5cb27dce2a

kwrooijen11:08:42

Ah ok, so this would apply for all types then

borkdude11:08:01

Well, that's up to your function, you can just pass through the object itself too, depending on the type

borkdude11:08:34

(fn [{:keys [:obj :loc]}] (if (map? obj) obj {:foo obj}))

borkdude11:08:05

Feel free to play with it and let me know if this works for you, before we merge it.

kwrooijen11:08:36

Right, currently I have this

(defn object-fn [{:keys [obj location]}]
  (cond
    (= String (type obj))
    (with-meta (->GDString obj) location)

    (= Long (type obj))
    (with-meta (->GDInt obj) location)

    (= Keyword (type obj))
    (with-meta (->GDKeyword obj) location)

    :else
    obj))
With the same idea, except that I was only applying it to the non meta type

kwrooijen11:08:16

I'll try checking out your commit

borkdude11:08:27

yeah, you can instead check if the object supports metadata and then do something different

kwrooijen11:08:15

That's also an option, but I need to check based on type anyway. So I think for my usecase this would be a bit more strict

borkdude11:08:34

@kevin.van.rooijen

user=> (parse-string "[1]" {:obj-fn (fn [{:keys [:obj :loc]}] (if (instance? clojure.lang.IObj obj) (vary-meta obj merge loc) [obj loc]))})
[[1 {:row 1, :col 2, :end-row 1, :end-col 3}]]

borkdude11:08:06

or:

user=> (defrecord Wrapper [obj loc])
user.Wrapper
user=> (parse-string "[1]" {:obj-fn (fn [{:keys [:obj :loc]}] (if (instance? clojure.lang.IObj obj) (vary-meta obj merge loc) (->Wrapper obj loc)))})
[#user.Wrapper{:obj 1, :loc {:row 1, :col 2, :end-row 1, :end-col 3}}]

kwrooijen11:08:53

Ah I see it also takes care of ::expected-delimiter 🙂

borkdude11:08:23

yeah, that was something I ran into

kwrooijen12:08:02

Seems to work like a charm 🙂

borkdude12:08:23

Reading nested objects also seems to work quite well:

user=> (parse-string "#{1 1}" {:obj-fn (fn [{:keys [:obj :loc]}] [obj loc])})
[#{[1 {:row 1, :col 3, :end-row 1, :end-col 4}] [1 {:row 1, :col 5, :end-row 1, :end-col 6}]} {:row 1, :col 1, :end-row 1, :end-col 7}]

borkdude12:08:08

lucky guess 😉

kwrooijen12:08:47

Tests still pass and my issue is fixed. Good enough for me 😛

borkdude12:08:02

I might rename the function and write some tests, then I'll merge to master, but for you it will probably only be a keyword change. I'll let it sink in a bit while I do some other stuff

kwrooijen12:08:22

Sounds good. Thanks a lot for your help!

borkdude14:08:31

@kevin.van.rooijen Merged to master. I believe the only change was that :obj-fn is now called :postprocess

🎉 3
borkdude14:08:04

@dominicm I believe you also asked for this feature once, about half a year ago maybe

dominicm14:08:24

I think I was looking at this for reader tags, so I could error gracefully.

borkdude14:08:09

edamame also has options for that I believe

borkdude14:08:30

I think you were looking at the metadata stuff for aero, for validating settings

borkdude14:08:59

since EDN etc doesn't have metadata in general but numbers, strings, etc do not carry metadata whatsoever

borkdude18:08:59

@kevin.van.rooijen btw, do you have a link to how you're using this, or is it closed source?

kwrooijen18:08:12

It's private at the moment. But I can open source it

borkdude18:08:33

I wonder if you postwalk the structure

kwrooijen18:08:37

Postwalking what exactly?

kwrooijen18:08:32

This is all I'm doing with it at the moment:

(defn obj-fn [{:keys [obj loc]}]
  (let [loc (-> loc
                (update :row dec)
                (update :end-row dec))]
    (cond
      (= String (type obj))
      (with-meta (->GDString obj) loc)

      (= Long (type obj))
      (with-meta (->GDLong obj) loc)

      (= Keyword (type obj))
      (with-meta (->GDKeyword obj) loc)

      (= Boolean (type obj))
      (with-meta (->GDBoolean obj) loc)

      :else
      (with-meta obj loc))))

borkdude18:08:30

alright. I do recommend using the built-in predicates like (int? ...), (boolean? ...) etc

kwrooijen18:08:56

Hm, I'm honestly wondering why I didn't do that 😄

kwrooijen18:08:24

Nice to have a code review every now and then

borkdude18:08:35

also (with-meta obj loc) will replace metadata on the obj, not sure if that's a problem in your tool

kwrooijen18:08:19

I don't think matter since I'm creating the records, right?

kwrooijen18:08:57

I mean it doesn't matter for me functionally speaking. But if I'm creating the record on the spot, which means they don't have any metadata

borkdude18:08:40

I was referring to the :else branch where you are not creating records

kwrooijen18:08:22

Ah right. I noticed that the values in the :else branch didn't have any metadata (in my case). But you're right. to be safe I should be using vary-meta instead

borkdude18:08:53

Do whatever you want, just something to note 🙂

kwrooijen18:08:17

I appreciate the feedback 👍