naomijub / edn-rs

[DEPRECATED]: Crate to parse and emit EDN
https://crates.io/crates/edn-rs
MIT License
81 stars 14 forks source link

Properly handle lisp quotes (' symbol) and numbers starting with '+' #108

Closed Grinkers closed 11 months ago

Grinkers commented 1 year ago

Fixes #105

codecov-commenter commented 1 year ago

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.24% :tada:

Comparison is base (bec6ea2) 71.06% compared to head (0c0e9fd) 71.30%.

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #108 +/- ## ========================================== + Coverage 71.06% 71.30% +0.24% ========================================== Files 8 8 Lines 940 941 +1 ========================================== + Hits 668 671 +3 + Misses 272 270 -2 ``` | [Files Changed](https://app.codecov.io/gh/naomijub/edn-rs/pull/108?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Julia+Naomi) | Coverage Δ | | |---|---|---| | [src/deserialize/parse.rs](https://app.codecov.io/gh/naomijub/edn-rs/pull/108?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Julia+Naomi#diff-c3JjL2Rlc2VyaWFsaXplL3BhcnNlLnJz) | `93.21% <100.00%> (+0.74%)` | :arrow_up: |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Grinkers commented 1 year ago

@naomijub

While doing this, I also ran into https://github.com/Grinkers/edn-rs/blob/symbols/src/deserialize/parse.rs#L69 which I believe should be (|c| !c.1.is_whitespace() && !DELIMITERS.contains(&c.1))

when doing tests, I ran into

---- deserialize::test::namespaced_maps_navigation stdout ----
thread 'deserialize::test::namespaced_maps_navigation' panicked at 'assertion failed: `(left == right)`
  left: `Nil`,
 right: `Key(":val")`', src/deserialize/mod.rs:665:9

 ---- deserialize::test::namespaced_maps stdout ----
thread 'deserialize::test::namespaced_maps' panicked at 'assertion failed: `(left == right)`
  left: `Key(":abc")`,
 right: `NamespacedMap("abc", Map({"0": Key(":val"), "1": Key(":value")}))`', src/deserialize/mod.rs:572:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

I can't find any reference to namespaced maps in EDN (or clojure, for that matter). Is this some sort of reader macro?

(clojure.edn/read-string ":abc{ 0 :val 1 :value}")
:abc

is what I would have expected and get.

naomijub commented 1 year ago

@Grinkers Our implementation of namespace mas is based off https://github.com/edn-format/edn#tagged-elements. It was adapting to Nubank's coding style at the time

naomijub commented 1 year ago

@Grinkers I will look this error later today and get back to you

naomijub commented 11 months ago

I'm really sorry for the time we took to review. We were both sick this past month