Fix initial path double slash in forgejo urls#205
Conversation
Pointed out in andir#204
libnpins/src/git.rs
Outdated
| } => format!("{}/{}/{}.git", server, owner, repo).parse()?, | ||
| } => { | ||
| let mut server = server.clone(); | ||
| server.set_path(&format!("{owner}/{repo}.git")); |
There was a problem hiding this comment.
While that does fix it, so far the code has instead normalized the strings within npins w.r.t. slashes etc., see e.g. line 277. I'd suggest sticking on one of the two, and then making our invariants more explicit maybe to prevent regressions.
There was a problem hiding this comment.
I'm not puzzling together how that is applicable here, since this is mutating a existing Url and so deals so with a presumably already normalized input?
There was a problem hiding this comment.
AFAICT the Url type is not normalized and merely a checked wrapper around some raw string backing with structured accessors, so all normalization would be up to npins to do upon url creation/storing. We mostly do this already, but not in all cases, so the suggestion would be to go all-in on that.
There was a problem hiding this comment.
So it turns out special urls always have a / path and can not be empty, so the url.set_path(""); in strip_url is actually a / path in all our use cases.
3e933a4 to
b982011
Compare
Pointed out in #204