Skip to content
This repository was archived by the owner on May 26, 2023. It is now read-only.

Use path.join instead of string concatenation #17

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

davidblurton
Copy link

Fixes two bugs.

  1. If public path doesn't end with a slash
  2. If asset path starts with . or ../

@lettertwo
Copy link
Owner

Firstly, sorry for not responding to this sooner! 😞

Secondly, please take a look at #10, and (hopefully) have a chuckle at how this PR is the inverse of that one 😝

Unfortunately, I can't pull this in because it will revert the fix for the Windows use case that #10 addressed. I'd like to have a solution here that solves for both that use case and the scenarios you're addressing here. I guess we need something that more-or-less implements joining and normalizing for urls (explicitly using '/' as a separator instead of the OS-specific file path separator, as path.join does).

Thanks for taking the time to open this, and i hope my slow response and subsequent rejection letter won't dissuade you from opening another PR, if you feel like taking a crack at solving for these use cases in a cross-platform way.

In the mean time, i'll leave this open as a reminder that we should do something about this.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants