tweag / nickel

Better configuration for less
https://nickel-lang.org/
MIT License
2.23k stars 85 forks source link

Add `new_from_sources` function to `Program` #1900

Closed ErinvanderVeen closed 2 months ago

ErinvanderVeen commented 2 months ago

This is similar to the new_from_path and new_from_paths function. A new_from_source function already existed, but a new_from_sources was still missing.

ErinvanderVeen commented 2 months ago

I think there's a bit of code duplication between new_from_sources and new_from_files. Although it's more salient here, there is in fact also a bit of code duplication between new_from_source and new_from_file.

It seems the only difference between from_source(s) and from_file(s) is that we use cache.add_source vs cache.add_file. After a quick look it doesn't seem reasonable to use cache.add_source generically because cache.add_file, although it does call to cache.add_source, also does more file-specific and non trivial operations like registering timestamps and stuff.

Do you think we could factor this out, by having a private generic function that e.g. takes a closure or just a bespoke enum SourceOrFile, and have new_from_source(s) and new_from_file(s) be simple wrappers around the corresponding underlying implementation?

Absolutely! I'll get on it!

ErinvanderVeen commented 2 months ago

We spoke before about how this is a good private function, but with a few minor tweaks, we could make it possible for users to mix input sources.

Topiary has a use for this actually. We want to create a single program from a Merge between the builtin source, and the configurations from the user (files). Our current solution is to read the files to a string, and create a cursor from that. Not ideal.

We would just have to expand the Input type to take another Type parameter:

enum Input<F, T, S> {
    /// A filepath
    Path(F),
    /// The source is anything that can be Read from, the second argument is the name the source should have in the cache.
    Source(T, S),
}

For use like: Input::<PathBuf, Cursor, String>.