chanced / jsonptr

JSON Pointer (RFC 6901) implementation for Rust
Apache License 2.0
44 stars 4 forks source link

Rework `Token` type #37

Closed asmello closed 2 months ago

asmello commented 2 months ago

This includes surprisingly few API changes. The main breaks come from the fact Token holds a (potentially non-static) lifetime now. I also remove several trait implementations and some methods, in favor of making the distinction between the encoded and decoded representations more explicit.

asmello commented 2 months ago

@chanced no urgency, but I think this is ready for a look when you have time.

chanced commented 2 months ago

@asmello, dude, you're awesome. This is so much cleaner than anything I would have produced.

I just realized I forgot to squash your last PR. Ah well.

asmello commented 2 months ago

I just realized I forgot to squash your last PR. Ah well.

There's always git rebase -i 😂

asmello commented 2 months ago

Ah, before I forget, one note since you mentioned you considered OnceCell — I assume that was so we could memoize the decoded value if it ever gets read. I don't think it's a bad idea, but I also think the Token type is a bit of an ephemeral type; people will typically produce it as part of iteration and either use it immediately or convert the decoded value to a String for keeping it around long-term.

chanced commented 2 months ago

It was. That logic is solid. I'm happy with out it.

chanced commented 2 months ago

The idea was to have something like`

pub struct Token(Internal);

enum Internal<'p> {
    Uncoded(Cow<'p, str>),
    Encoded { decoded: OnceCell<String>, encoded: Cow<'p, str> }
}