search

pchampin / sophia_rs

Sophia: a Rust toolkit for RDF and Linked Data
Other
214 stars 23 forks source link

making the namespace! macro public #33

Closed dwhitney closed 4 years ago

dwhitney commented 4 years ago

Hi, I'd love to use the sophia::ns::namespace! macro for my own projects, but I had to make a couple of changes to the code to make it work. I suspect making the fields on IriData public is not something you desire, but perhaps we can have a discussion about how to make this work? (btw, I am a neophyte Rust programmer)

pchampin commented 4 years ago

Hi, thanks for your interest in sophia.

Thanks also for spotting this bug (namespace! not being usable outside sophia). It made me realize a deeper issue about exposing this macro: it is conceptually unsafe. More precisely, it statically builds Iris, without any check that they are valid. I prefer to keep it that way to avoid an overhead caused by the ns submodules, but that should be documented.

The solution to your problem (that does not involve publicly exposing IriData fields) would be to create a new constructor pub const unsafe IriData::from_raw_parts, and call this constructor in ns_tem! instead of setting the fields directly.

btw, I am a neophyte Rust programmer

I've been there not so long ago, and honestly, I still have much to learn ;-) Welcome onboard.

pchampin commented 4 years ago

Oh, piece of advice: never make a pull request with your master branch. If your pull request is rejected (and this one might well be...) your master branch becomes irreconcilable with the origin repository... You should always create a dedicate branch for a pull request.

pchampin commented 4 years ago

commit 3ce6c6c implements the better approach suggested above

But again, thanks for spotting this and hinting at a solution :-)

dwhitney commented 4 years ago

Thanks!