Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow relative urls in manifest in WebPackageResolver #15

Merged
merged 4 commits into from
Sep 12, 2018

Conversation

pipe01
Copy link
Contributor

@pipe01 pipe01 commented Aug 24, 2018

If there's a line like 1.2.3 ./build.zip in http://www.example.com/manifest.txt, the archive URL will be set to http://www.example.com/build.zip.

@Tyrrrz Tyrrrz added the enhancement New feature or request label Aug 24, 2018
@Tyrrrz
Copy link
Owner

Tyrrrz commented Aug 24, 2018

Hi. Nice improvement, this is actually really useful.
Maybe you can use Uri and use its Relative tag to do this with even less additional code?

@pipe01
Copy link
Contributor Author

pipe01 commented Aug 24, 2018

Is that what you mean?

@Tyrrrz
Copy link
Owner

Tyrrrz commented Aug 25, 2018

I meant more as:

  1. Convert _manifestUrl to Uri and store in a var
  2. Parse the package URL and create a relative URI based on the manifest URI. This will take care of all sorts of relative URLs automatically.

@pipe01
Copy link
Contributor Author

pipe01 commented Aug 26, 2018

That's basically what I'm doing, but without the intermediate variable.

@Tyrrrz
Copy link
Owner

Tyrrrz commented Aug 27, 2018

Not exactly, since you wouldn't need to call StartsWith and bother with relative paths. 😛

@Tyrrrz Tyrrrz changed the title Replace ./ with manifest directory in WebPackageResolver Allow relative urls in WebPackageResolver Sep 12, 2018
@Tyrrrz Tyrrrz changed the title Allow relative urls in WebPackageResolver Allow relative urls in manifest in WebPackageResolver Sep 12, 2018
@Tyrrrz Tyrrrz merged commit db8512b into Tyrrrz:master Sep 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants