Skip to content

Conversation

Grinkers
Copy link
Owner

@Grinkers Grinkers commented Jul 4, 2025

Fixes #2

@coveralls
Copy link

coveralls commented Jul 4, 2025

Coverage Status

coverage: 97.732% (-0.009%) from 97.741%
when pulling cf56974 on fix-namespace-parsing
into 0d121b6 on main.

@cap10morgan
Copy link

I tried out a few things in the REPL:

~> clj
Clojure 1.12.1
user=> {:foo "bar" :baz "qux"}
{:foo "bar", :baz "qux"}
user=> #:thingy{:foo "bar" :baz "qux"}
#:thingy{:foo "bar", :baz "qux"}
user=> #:thingy{:foo "bar" :other/baz "qux"}
{:thingy/foo "bar", :other/baz "qux"}
user=> #:thingy{:foo "bar" :other/baz "qux" :quux "I forgot the next one"}
{:thingy/foo "bar", :other/baz "qux", :thingy/quux "I forgot the next one"}
user=> #:thingy {:foo "bar" :other/baz "qux" :quux "I forgot the next one"}
{:thingy/foo "bar", :other/baz "qux", :thingy/quux "I forgot the next one"}

So yeah, it seems spaces are OK. The only other variant it might be good to make sure your code covers is the non-default namespaced key where it should preserve the key's namespace and not impose the default (the :other/baz in my REPL session above).

@Grinkers Grinkers force-pushed the fix-namespace-parsing branch 2 times, most recently from 4ea7aff to bbf95d7 Compare July 11, 2025 19:13
@Grinkers
Copy link
Owner Author

user=> #:thingy{:foo "bar" :other/baz "qux" :quux "I forgot the next one"}
{:thingy/foo "bar", :other/baz "qux", :thingy/quux "I forgot the next one"}

I don't think it's actually possible for this crate to allocate a new struct with resolved namespaces like that without causing huge breaking changes and changing the way memory is allocated. This requires creating a new allocated String to create the &str reference, which would potentially cause issues on no_std environments.

All the data is read and put into a usable structure though. What I did do is add a convenient get for when traversing a Tagged type which I believe matches Clojure's get. What do you think?

fn namespace_get() {
// (def edn-data (edn/read-string "#:thingy {:foo \"bar\" :baz/bar \"qux\" 42 24}"))
let edn_data = edn::read_string(r#"#:thingy {:foo "bar" :baz/bar "qux" 42 24}"#).unwrap();
// (get edn-data 42) -> 24
assert_eq!(edn_data.get(&Edn::Int(42)), Some(&Edn::Int(24)));
// (get edn-data :foo) -> nil
assert_eq!(edn_data.get(&Edn::Key("foo")), None);
// (get edn-data :thingy/foo) -> "bar"
assert_eq!(edn_data.get(&Edn::Key("thingy/foo")), Some(&Edn::Str("bar")));
// (get edn-data :baz/bar) -> "qux"
assert_eq!(edn_data.get(&Edn::Key("baz/bar")), Some(&Edn::Str("qux")));
}

@cap10morgan
Copy link

Seems like it works then! Does this cover things like End::Map.contains_key as well?

@Grinkers Grinkers force-pushed the fix-namespace-parsing branch from 1aa9bfa to 1743155 Compare July 11, 2025 21:58
@Grinkers
Copy link
Owner Author

Seems like it works then! Does this cover things like End::Map.contains_key as well?

Tagged(&'e str, Box<Edn<'e>>) essentially just are a couple references to the original data, not realizing/processing the data. I have get and nth because they're the most common to quickly get data out. I implemented contains

// (contains? edn-data 42) -> true
assert!(edn_data.contains(&Edn::Int(42)));
// (contains? edn-data "42") -> false
assert!(!edn_data.contains(&Edn::Str("42")));
// (contains? edn-data :foo) -> false
assert!(!edn_data.contains(&Edn::Key("foo")));
// (contains? edn-data :thingy/foo) -> true
assert!(edn_data.contains(&Edn::Key("thingy/foo")));
// (contains? edn-data :baz/bar) -> true
assert!(edn_data.contains(&Edn::Key("baz/bar")));
// (contains? edn-data :bar/baz) -> false
assert!(!edn_data.contains(&Edn::Key("bar/baz")));

The underlying data can be processed in any way you want, and hopefully contains assists in making it easier. Rust structs are more limiting than dynamic evaluated lisp (can't use '/' in field names, what would a namespace even mean for a key, etc).

For data that rust can represent, we already have fully automatic Serde:

#[derive(Serialize, Deserialize, PartialEq, Debug)]
#[serde(rename_all = "kebab-case")] // Clojure mostly uses kebab-case
struct Test {
int: u32,
v: Vec<u32>,
#[serde(alias = "foo_bar")] // This will allow :foo_bar to also work
foo_bar: u8,
b: bool,
: &'static str,
}
fn main() {
let test = Test { int: 1, v: vec![1, 2, 3, 42], foo_bar: 32, b: false, : "silly" };
let expected = r#"{:int 1, :v [1 2 3 42], :foo-bar 32, :b false, :猫 "silly"}"#;
let ser = to_string(&test).unwrap();
assert_eq!(ser, expected);
let deser = from_str::<Test>(expected).unwrap();
assert_eq!(deser, test);
}

The test suite should show examples of everything that's possible
https://github.com/Grinkers/clojure-reader/blob/b00e51e4782b27856d9490ad2ac35320d975f13a/tests/de.rs
https://github.com/Grinkers/clojure-reader/blob/b00e51e4782b27856d9490ad2ac35320d975f13a/tests/ser.rs

@cap10morgan
Copy link

Sorry for my delayed responses. I'm in the middle of an international move. I will try this out as soon as I can!

@Grinkers Grinkers force-pushed the fix-namespace-parsing branch 3 times, most recently from efbb0f6 to fc11cca Compare July 23, 2025 01:00
@Grinkers
Copy link
Owner Author

Added a lot more test coverage trying to hit edge cases, as well as implemented contains for lists, vectors, and sets too.

I will bump to 0.4 after this, if there's no issues.

@Grinkers Grinkers force-pushed the fix-namespace-parsing branch from fc11cca to b1ed736 Compare July 23, 2025 01:26
@Grinkers Grinkers force-pushed the fix-namespace-parsing branch from b1ed736 to 133484f Compare July 23, 2025 21:16
@Grinkers Grinkers force-pushed the fix-namespace-parsing branch from 50f514b to cf56974 Compare July 25, 2025 23:25
@Grinkers Grinkers merged commit fe5b073 into main Jul 27, 2025
32 checks passed
@Grinkers Grinkers deleted the fix-namespace-parsing branch July 27, 2025 01:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Default namespaces cause maps to end parsing early
3 participants