libpathrs
libpathrs copied to clipboard
make libpathrs Rust configuration sane
The current method of configuration is a really bad idea (exposing structure internals of Root
is going to cause problems, I can feel it in my bones). So we should instead use a different design, but it's unclear to me what the best approach is:
-
Just have methods to set (and possibly get) configuration of the object. It's not the nicest thing in the world, but it's fairly workable.
-
The most common approach is the "builder" method. The issue with it is whether there should be a way to modify the configuration after you've consumed the builder. If yes, then what is the point of the builder (you could just modify the configuration and not have a builder -- which is the alternative proposal). If no, and then how do we expose the builder method through the C API -- we'd need to have C builders which is just going to be absolutely awful.
Some follow-on questions (and my tentative answers):
-
Do people really care about modifying the configuration after the fact?
- Maybe, I'm not too sure.
-
How do we deal with the
try_clone
case and modifying the configuration of the copy (ditto forfrom_file_unchecked
)?- We could do it by making the builder take a variety of "source" arguments (so you can source from a directory -- creating a new builder -- or from an existing object or file handle). The configuration could be pre-loaded from the existing object if applicable. Downside is that the API would be something like:
let root = RootBuilder::new().source("/some/path").build()?;
let root = RootBuilder::new().source_file_unchecked(root_file).build()?;
// how do we do try_clone?
let root = old_root.try_clone()?.build()?; // ?!
let root = RootBuilder::new().source_root(old_root).build()?; // !?
let root = RootBuilder::new().source_file_unchecked(old_root.try_clone()?.into_file()).build()?; // lolno
Or maybe you could make the API for try_clone
be the same as it currently is, but there is a way you can consume a Root
(or Handle
) and turn it into a builder?
let root = old_root.try_clone()?.rebuild().set_some_config().build()?; // maybe?
The other problem is what should Root::resolve
return? A builder? Does that really make too much sense? Handle
s don't really have a nice point where you can insert a constructor-like setup...