URI-based dependency parsing #678
Conversation
bcardiff
merged commit 91c3656
into
crystal-lang:dependency-cli-parsing
| case value | ||
| when .starts_with?("./"), .starts_with?("../") | ||
| Parts.new("path", Path[value].to_posix.to_s) | ||
| when .starts_with?("git@") |
There was a problem hiding this comment.
Edge case: it's the username to connect with, it doesn't have to be git.
There was a problem hiding this comment.
Yes of course. This is just an example for a heuristic to derive a resolver from a schemeless URI. If the username is git, it's likely a git repo.
We can do similar things for other URI components. For example, if the hostname starts with fossil., it's likely a fossil repo. If the path ends with .git it's likely a git repo.
I'm not entirely sure if we should do all those things when it's relatively simple to just provide a fully qualified URI. But it seems easy enough, shouldn't cause much issues and might improve UX in some cases.
| expect_parses("file://#{local_absolute}", "path", local_absolute, Any) | ||
| # Path resolver syntax | ||
| expect_parses("path:#{local_absolute}", "path", local_absolute, Any) | ||
| expect_parses("path:#{local_relative}", "path", local_relative, Any) | ||
| # Other resolvers short | ||
| expect_parses("git:git://git.example.org/crystal-library.git", "git", "git://git.example.org/crystal-library.git", Any) | ||
| expect_parses("git+https://example.org/foo/bar", "git", "https://example.org/foo/bar", Any) | ||
| expect_parses("git:https://example.org/foo/bar", "git", "https://example.org/foo/bar", Any) |
There was a problem hiding this comment.
There are no specs for mercurial/hg or fossil URL.
| else | ||
| Parts.new("git", uri.path) | ||
| end | ||
| when "git+https" |
There was a problem hiding this comment.
Why treat git particularly, instead of <resolver>+ for fossil and mercurial?
Edge cases: what about other schemes and resolver combinations, like git+file:, fossil+http:, or hg+ssh://?
There was a problem hiding this comment.
Yes, of course. That all sounds good. This patch was really just intended to demonstrate the concept as an alternative to the original approach in #673. It certainly isn't production ready.
|
I'm still not sure what are the acceptable formats, despite the method specs. I feel like
Frankly, that's far too many cases already 😞 |
|
Yes, the URIs combining a resolver name with an opaque url is also coherent with SPEC. This does not apply only to shorthands. It's nice that you can always use the same notation as in We can easily generalize the mapping implementation and get rid of any special cases (see https://github.com/crystal-lang/shards/pull/673/files#r2102181386). I'm happy to drop all (or most) of these heuristics for the MVP. They're certainly not essential, only nice to have for a neat UX. |
This proposes an alternative implementation for #673