Add support for packages to nickel-lang-core #2094
Conversation
| /// | ||
| /// Like [`normalize_abs_path`], this works only on the path itself | ||
| /// (i.e. not the filesystem) and does not follow symlinks. | ||
| pub fn normalize_rel_path(path: &Path) -> PathBuf { |
There was a problem hiding this comment.
I needed this utility function in the upcoming nickel_lang_package crate. Putting it next to normalize_abs_path seemed reasonable, but it doesn't actually need to be in this crate.
|
Bencher Report| Branch | npm-core |
| Testbed | ubuntu-latest |
Click to view all benchmark results
| Benchmark | Latency | nanoseconds (ns) |
|---|---|---|
| fibonacci 10 | 📈 view plot 🚷 view threshold | 500,150.00 |
| foldl arrays 50 | 📈 view plot 🚷 view threshold | 1,653,500.00 |
| foldl arrays 500 | 📈 view plot 🚷 view threshold | 6,673,700.00 |
| foldr strings 50 | 📈 view plot 🚷 view threshold | 6,995,800.00 |
| foldr strings 500 | 📈 view plot 🚷 view threshold | 61,136,000.00 |
| generate normal 250 | 📈 view plot 🚷 view threshold | 40,879,000.00 |
| generate normal 50 | 📈 view plot 🚷 view threshold | 1,881,600.00 |
| generate normal unchecked 1000 | 📈 view plot 🚷 view threshold | 3,347,200.00 |
| generate normal unchecked 200 | 📈 view plot 🚷 view threshold | 760,570.00 |
| pidigits 100 | 📈 view plot 🚷 view threshold | 3,218,800.00 |
| pipe normal 20 | 📈 view plot 🚷 view threshold | 1,500,100.00 |
| pipe normal 200 | 📈 view plot 🚷 view threshold | 9,880,100.00 |
| product 30 | 📈 view plot 🚷 view threshold | 822,090.00 |
| scalar 10 | 📈 view plot 🚷 view threshold | 1,516,900.00 |
| sum 30 | 📈 view plot 🚷 view threshold | 805,430.00 |
| /// A table mapping FileIds to the package that they belong to. | ||
| /// | ||
| /// Path dependencies have already been canonicalized to absolute paths. | ||
| package: HashMap<FileId, PathBuf>, |
There was a problem hiding this comment.
Nitpick: what about packages or package_map? A singular package being a HashMap<_, _> is a tad surprising IMHO
| /// A path or a package, like the name says. | ||
| /// | ||
| /// Paths come with a choice of import formats. Packages are always in nickel format, for now. | ||
| pub enum PathOrPackage<'a> { |
There was a problem hiding this comment.
Nitpick: maybe we can find a more abstract/generic name? One day if we allow for bare URLs for example? (probably won't, but let's imagine). Maybe ImportLoc, ImportSpec, AbstractLoc, ImportPath, or something better.
| @@ -209,6 +209,9 @@ pub enum Term { | |||
| #[serde(skip)] | |||
| Import { path: OsString, format: InputFormat }, | |||
|
|
|||
| #[serde(skip)] | |||
| ImportPkg(Ident), | |||
There was a problem hiding this comment.
Although we are getting rid of Term, adding a new variant impacts quite a few places with pattern matching where we don't care about imports. Should we just rename PathOrPackage to something like Import, taking an owned OsString, and use that type as the parameter of Term::Import? After all the'yre all imports. What do you think?
These are the changes to the core crate, picked out from the
npmbranch.Essentially, this enables the
import pkgsyntax, and introducesPackageMapfor figuring out how to interpret those import statements.It also includes deserialization support for enum variants, because I ran into that while working on manifest files. That's unrelated to most of the changes here, so I could break it out as a separate PR if you want.