Resolution handler #1068
Resolution handler #1068
Conversation
g-r-a-n-t
force-pushed
the
resolution-handler
branch
2 times, most recently
from
a7ce31a to
a61d9e9
Compare
g-r-a-n-t
force-pushed
the
resolution-handler
branch
5 times, most recently
from
fd31a02 to
06381f9
Compare
g-r-a-n-t
force-pushed
the
resolution-handler
branch
3 times, most recently
from
ec49518 to
5430d93
Compare
g-r-a-n-t
force-pushed
the
resolution-handler
branch
2 times, most recently
from
685f878 to
1069810
Compare
g-r-a-n-t
force-pushed
the
resolution-handler
branch
3 times, most recently
from
f28fd37 to
d0b347d
Compare
crates/common/src/config.rs
Outdated
| }, | ||
| #[derive(Debug, Clone, PartialEq, Eq, Hash)] | ||
| pub struct DependencyDescription { | ||
| pub url: Url, |
There was a problem hiding this comment.
This might not be expressive enough to handle both relative and absolute dependency paths.
| } | ||
|
|
||
| #[derive(Debug, Clone)] | ||
| #[derive(Debug, Clone, PartialEq, Eq, Hash)] | ||
| pub struct Dependency { |
There was a problem hiding this comment.
It could be really nice if Dependency is just a flat thing, something like:
Dependency {
alias: SmolStr,
location: Either<Url, Utf8PathBuf>,
arguments: Option<IngotArguments>
}
crates/common/src/ingot.rs
Outdated
| pub fn config(self, db: &'db dyn InputDb) -> Option<Config> { | ||
| db.workspace() | ||
| .containing_ingot_config_file(db, self.base(db)) | ||
| .map(|config_file| Config::from_string(config_file.text(db).clone())) | ||
| } |
There was a problem hiding this comment.
Here's where the magic happens, where we just parse the config from the config's File input into a Config object. I added all the derivations to Config so that it can be salsa tracked.
crates/resolver/src/ingot.rs
Outdated
| @@ -47,16 +47,23 @@ impl ResolutionHandler<FilesResolver> for BasicIngotNodeHandler { | |||
| return config | |||
| .dependencies | |||
| .into_iter() | |||
| .map(|dependency| match dependency.description { | |||
| DependencyDescription::Path(path) => ( | |||
| ingot_path.join(path).canonicalize_utf8().unwrap(), | |||
There was a problem hiding this comment.
Seems like the resolver was handling relative paths quite well before. If Dependency/DependencyDescription can only store a Url this gets trickier.
There was a problem hiding this comment.
I don't think the dependency struct should hold onto a url, unless it is a git repo or something. it should just be a simple representation of what is in the config file. it's pretty easy for a function in common::ingot to map a path DependencyDescription to a file url given the parent ingot.
crates/hir/src/hir_def/mod.rs
Outdated
| .filter_map( | ||
| |Dependency { | ||
| alias, | ||
| description: DependencyDescription { url, .. }, |
There was a problem hiding this comment.
The oversimplified DependencyDescription with its handy Url is making this a little too easy. In the end we need to be able to consistently map a Dependency to a URL. In the case of relative path dependencies, there needs to be some way of injecting a base URL so that if the dependency points to a relative path we can resolve an absolute URL for that relative path.
Maybe we can have a method like:
impl Dependency {
fn url(&self, base_url: Url) -> Url {
let flattened_absolute_url = match self.location ... // handle path vs url
return flattened_absolute_url
}The idea would be that the dependency itself carries around either a url or a path and that this can be resolved given a base url.
crates/common/src/ingot.rs
Outdated
| } | ||
|
|
||
| #[salsa::tracked] | ||
| pub fn dependencies(self, db: &'db dyn InputDb) -> Vec<Dependency> { |
There was a problem hiding this comment.
this could just return the aliases and urls and the dependency descriptions can remain relative
g-r-a-n-t
force-pushed
the
resolution-handler
branch
from
caa2daf to
f97a007
Compare
g-r-a-n-t
force-pushed
the
resolution-handler
branch
from
f97a007 to
bee0e97
Compare
g-r-a-n-t
force-pushed
the
resolution-handler
branch
from
109a9dd to
2154b82
Compare
g-r-a-n-t
force-pushed
the
resolution-handler
branch
from
2154b82 to
6b7285d
Compare
No description provided.