Fork me on GitHub
#clj-yaml2022-09-17
>
lread21:09:47

Heya, starting to feel like I’m bogging down your https://github.com/clj-commons/clj-yaml/pull/38 @grzm!

lread21:09:14

Probably easier to discuss here than on a GitHub issue.

lread21:09:30

I guess my main question is: do we really need/want to expose a snakeyaml constructor factory? My inclination is no.

grzm22:09:35

No worries. I'm just busy and exhausted.

grzm22:09:08

trying to think of how tags should be exposed. As meta is probably the easiest way (though not all Yaml node values can take meta). tagged-literals would be another possible way. Then then question is how to translate the tag representation to a tagged literal representation. And I don't know what round-tripping would look like.

lread22:09:15

Thanks! When you have recharged your batteries (not before that), would love to chat.

grzm22:09:28

I'm sympathetic to the idea of not exposing snakeyaml implementaiton details. That said, the current options are already not ideal: you can provide both :unsafe and :mark keyword args, but they're not really fully orthogonal. (I'm trying to look past my dislike for keyword args: that'd be a version 2 change I'd like to see)

lread22:09:28

Good point about meta… hmmm…

lread22:09:11

Well, get some good rest, and we can percolate on it.

grzm22:09:11

I suppose we could change tagged nodes to a map with :clj-yaml/tag and :clj-yaml/value keys (namespaced to ensure disambiguation with the data itself)

lread22:09:07

Oh that’s why :mark is optional… can’t do meta so it returns maps?

grzm22:09:09

I'd like to see a full-on critique of EDN. Other serializations have such obvious flaws, but I'm sure I'm biased and blind to issues with EDN

grzm22:09:29

re: :mark Oh, right. The examples in the README clearly show how the MarkedConstructor is supposed to be used. How'd I miss that?

lread22:09:35

Oh… :mark wraps with with java class, same idea tho.

grzm22:09:52

(apologies for the snark 😛)

lread22:09:23

YAML is not a snark free zone. So snark away!

grzm22:09:03

The map thing might be a way forward. Questions about how to only change the ones that would otherwise fail…prolly try/catch or something…

grzm22:09:00

goes to get something to eat

lread22:09:32

But wouldn’t a map be a breaking change?

grzm22:09:14

Right now it just throws an exception with unknown tags, so (a) it's already broken and (b) any of this I'd see added as an option, like pass through but not throwing away anything.

lread22:09:54

So if you’d enabled parsing unknown tags, you’d be in a different mode where maps are returned?

(parse-string "!!bad-tag 3" {:read-unknown-tags true})
Might return?

grzm22:09:30

Something like that, yeah

lread22:09:54

Maybe if we have an option where clj-yaml can return nodes instead of values? That might satisfy both :mark and :read-unknown-tags and also give you a richer return? I dunno, just throwing ideas out.

lread22:09:21

(And nodes might be maps)

grzm22:09:46

Or sequences. (Vectors/arrays)

lread23:09:33

Forgetting about :mark for a sec, and round-tripping, another idea would be to return separate warnings/findings. Ex. {:type :unknown-tag :row x :col y}

grzm23:09:42

I wouldn't want a generic node one. I'm not against it, but my use case is that tags are the exception, not the rule.

grzm23:09:14

I just don't want the parser to bail if there's a tag

lread23:09:28

Right. I guess we should step back and ask if it is important to know that an unrecognized tag was read. Round-tripping support would be one reason. As a user do you otherwise care?

lread23:09:34

We can always have documented limitations. Fine with me if they are reasonable to clj-yaml users.

lread23:09:33

Note: :mark result is currently not directly round-tripable:

(def x (parse-string "3" :mark true))
(generate-string x)
;; => "start: {line: 0, index: 0, column: 0}\nend: {line: 0, index: 1, column: 1}\nunmark: 3\n"

borkdude10:09:42

@UE21H2HHD note that "don't bail on unknown tags" is the primary use case for the PR, which is just a feature coming over from another clojure library. That feature has been more or less directly ported. Since that feature worked fine in that other library, I was ok with the PR as is. To limit exposure, we could change that to a toggle or so, to behave the same as with the explicit :constructor.

borkdude10:09:36

off-topic: I just had a "don't bail on unknown tags" problem for bb.edn https://github.com/borkdude/quickdoc/issues/19

lread16:09:46

Cool, so sounds like knowing what tags were unknown doesn't have to be part of this particular issue.

borkdude21:09:02

Thanks for creating this channel! I'll go to sleep now but when I come back, I'll read up.. 💤

❤️ 1
💤 1