Fork me on GitHub
#rewrite-clj
<
2020-11-28
>
lread17:11:38

I am wondering about namespaced maps and sexpr behaviour on affected map keys. Given:

#:my-prefix{:x 1}
When we perform an sexpr on the map-node we would expect:
{:my-prefix/x 1}
But when we perform an sexpr on the individual contained keyword-node for :x, I’m guessing that a caller might expect the context to be taken into account and see a return of:
:my-prefix/x
Rather than what I have rewrite-cljc (WIP) currently return:
:x
Before I rabbit-hole too much, wondering what other folks think here. Perhaps it is acceptable to document a caveat, that if you want map keys to be affected by their namespaced map, to sexpr at the map-node rather than the key-node level?

sparkofreason17:11:56

I'd vote caveat. For me, at least, the utility of rewrite-cljc is to manipulate code as written, rather than as evaluated.

lread17:11:02

Thanks @U066LQXPZ, one concern is that rewrite-clj uses sexpr internally in zip API’s find-value, find-next-value and get. I suppose I could caveat for those too.

sparkofreason17:11:07

Huh, the use of sexpr there is interesting, since it gives you something more like the evaluated result, after reader dispatch etc. has been applied.

lread17:11:56

Yeah, rewrite-clj’s sexpr, as I see it, is about convenience. I think the idea was that it is often more convenient to work with Clojure forms than rewrite-clj nodes. In the other direction we have convenient coercion from Clojure forms to rewrite-clj nodes.

sparkofreason19:11:11

Functions like find-value are definitely a convenience and a nice abstraction, and covers a lot of cases. There's just going to be some corner cases like this where the user will have to be aware of the differences, because not all clojure syntax has a direct representation as an s-expression. I notice, for example, that (z/sexpr (z/of-string "#inst \"2020-01-01\"")) gives (read-string "#inst \"2020-01-01\"").

lread16:11:30

Yeah, good point. About those read-string variants, I guess if the caller wants, they can then eval the read-string expression? You might assume I should know, but as I don’t have access to the original author of rewrite-clj, I make guesses, and ponder with others in this channel. :man-shrugging:

lread16:11:05

@U066LQXPZ I forgot I had raised an issue to document read-string returns. Added your example. https://github.com/lread/rewrite-cljc-playground/issues/53

3
lread17:11:50

As for the original discussion about losing namespaced map context when sexpr-ing on an individual key in the map… There might be something I can do, but I think I’ll just raise a git issue to maybe (or maybe not) address later… and document the limitation. @U04V15CAJ would this work for you too?

borkdude17:11:20

@UE21H2HHD I'm not sure what to do here. I think supporting this will maybe complicate things since the nodes are no longer context-free?

borkdude17:11:12

An alternative might be to make the keys namespaced as well and store the namespaced map as such

borkdude17:11:55

@UE21H2HHD I think that may not be a bad idea. I'm not sure what the consequences are for restoring the code to string from such a change

lread17:11:14

The issue is only for sepxr. So kind of a special case. Are you suggesting the keyword would optionally also carry the namespaced map prefix and auto-resolved? values?

lread17:11:51

The zipper is aware of the tree and knows the parent. I suppose it could optionally pass in the parent node info when sexpr ing a keyword-node that is in a namespaced map. But that might be a bit gross.

lread17:11:59

Alternatively the parser could add the namespaced map context to the keyword node. But then we’d have that same data in two places, perhaps making node update confusing.

borkdude17:11:13

@UE21H2HHD I was suggesting that the keyword node would just be literally :foo/bar represented as a normal rewrite-clj keyword

borkdude17:11:58

sexpr-ing that would then automatically work

borkdude17:11:16

and also these nodes are more consistent to look at outside the context of the map

lread17:11:47

How would that work for folks who just want to reformat source code?

#::myalias  {:x 1 :y 2}
We’d still need to have keyword-node represent :x as :x no?

borkdude17:11:48

@UE21H2HHD I think this would actually simplify things. What if you would add an un-namespaced key? Then you would have to print the map differently anyways. So when printing a namespaced map, one should check if the map is still namespaced and if all the children are still namespaced accordingly.

lread17:11:45

I’m not following… yet! simple_smile A tool like cljfmt only wants whitespace affected. Non-whitespace code remains as written by the author. If the author wrote:

#:booya {:x 1 :booya/y 2}
This is equivalent to, but not as originally written:
#:booya{:x 1 :y 2}
Would your idea support that preserving the originally written code? (Forgive me if I’ve totally misunderstood your idea).

borkdude17:11:40

I understand that this is the case for cljfmt, but not for refactoring tools

borkdude17:11:54

e.g. the editor that @U066LQXPZ is going to make, might manipulate keys of a map

sparkofreason17:11:09

This seems no different than editing clojure code. If you had the map #::myalias {:x 1 :y 2} and wanted to add un-namespaced :z, you'd have to edit the code to be {::myalias/x 1 :myalias/y 2 :z 3}. Those two things should parse to different AST's. How about a function to transform between the two AST node types?

borkdude17:11:07

@U066LQXPZ I'm talking about the case where you first parse the first example, then manipulate the keys code-wise and then write out the map. I'm not even sure if that works correctly today.

borkdude17:11:28

So if you store the keys always in a normalized fashion, you don't have to bother looking at the properties of the parent map. The printing of keys should always work independently from the parent map. And when printing the parent map, one can do a simple check: 1) the map was namespaced and 2) the children are all still namepaced, then write the prefix form, else write a normal map with individual keys

lread17:11:17

So, would rewrite-cljc rewrite this:

{:foo/x 1 :foo/y 2}
to this?:
#:foo{:x 1, :y 2}

borkdude17:11:36

no, because the parent map of the keys is not namespaced

borkdude17:11:56

you would only write the second if the original map node was already namespaced

sparkofreason17:11:36

(-> (z/of-string "#:foo{:doink 1}")
     z/down
     z/right
     (z/append-child (n/keyword-node :bar))
     (z/append-child (n/value 2))
     (z/print-root))
=>
#:foo{:doink 1 :bar 1}

borkdude17:11:50

yeah, that's wrong

borkdude17:11:21

also getting the key :doink from the children and calling sexpr on it would be wrong right now

sparkofreason17:11:34

It's wrong in the sense of how the Clojure compiler would interpret it, right in the sense of being a literal translation of what was typed. For example, the clojure compiler would not accept the form ` ` (p/parse-string "{:foo 1 :bar}")

borkdude17:11:14

sexpr should return not what is typed, but what is semantically the correct representation in clojure

borkdude17:11:51

to return what is typed you can already use str

sparkofreason17:11:44

Yes, I agree that sexpr should handle things differently. And in general, it may be useful to have functions that transform nodes from a literal representation of the text to something closer to what the compiler actually sees after the reader does its thing. sexpr could probably leverage that to be more consistent.

borkdude17:11:30

Does (sexpr namespaced-map) give the right result right now?

lread18:11:53

Sorry, had to run an errand… This is a great discussion, thanks to you both! Forgetting about sexpr or a moment, I think it is important that what rewrite-cljc writes out what was written by the original author of the parsed code (with the exception of newlines which might be converted). I don’t mean semantically equivalent, I mean what the author typed in at the keyboard. This should, ideally, return true for any source we throw at rewrite-cljc:

(def source (slurp ""))
(= source (-> source z/of-string z/root-string))
@U04V15CAJ, will this hold true with your idea?

borkdude18:11:30

Yes, it should work

lread18:11:37

Cool, thanks @U04V15CAJ, that helps me to understand your angle better. I’ll dig in and explore your idea more deeply.

lread19:11:36

@U04V15CAJ, with your idea, would this rewrite as it was written?

#:foo {:x 1 :foo/y 2}
if not, I suppose having rewrite-cljc remember that y was qualified with foo when parsed would work.

borkdude19:11:33

@UE21H2HHD that's an edge case I haven't thought about. This is another edge case I didn't know was possible:

#:foo {:x 1 :bar/y 2}
{:foo/x 1, :bar/y 2}

lread19:11:22

There’s a really good code snippit in https://clojure.atlassian.net/browse/CLJ-1910 that describes namespaced map behaviours.

lread19:11:54

Until I read its examples, I did not know about the _ prefix:

;; effects on keywords with ns, without ns, with _ ns, and non-kw
user=> #:foo{:kw 1, :n/kw 2, :_/bare 3, 0 4}
{:foo/kw 1, :n/kw 2, :bare 3, 0 4}

lread19:11:48

afk for a bit…

borkdude19:11:17

that seems like a major hack ;)

borkdude19:11:19

@UE21H2HHD I'm happy to learn edamame parses this correctly:

user=> (e/parse-string "#:foo{:kw 1, :_/bar 3}")
{:bar 3, :foo/kw 1}
because it relies on tools.reader for this 😅

lread20:11:55

Ha! I guess it is not a horrible way to support unqualified keys in a namespaced map?

lread20:11:07

Just probably not well known

borkdude20:11:25

I don't think namespaced maps are really that widely used anyways

lread21:11:39

Anyhoo, I’ll sleep on all this and take a look again tomorrow. If nothing obvious and good comes to mind, I’ll just defer the work to a git issue.

lread21:11:49

Just so I can get some momentum of some sort! simple_smile

borkdude21:11:08

Most of those hits seem to show either tests for clojure itself or related tooling, or Datomic-related stuff

borkdude21:11:43

Have a good night