Skip to content

ignore url query strings in file path#3

Open
alancmclean wants to merge 2 commits intombostock:masterfrom
alancmclean:master
Open

ignore url query strings in file path#3
alancmclean wants to merge 2 commits intombostock:masterfrom
alancmclean:master

Conversation

@alancmclean
Copy link

URL Query strings are included in the file path causing "file not found" errors. This should workaround that. Nothing fancy, just a split on the first ? encountered in the file path.

@mbostock
Copy link
Owner

Better would be something like this:

function defaultFile(url) {
  var i = url.indexOf("/", 1) + 1,
      j = url.indexOf("?", i + 1);
  return decodeURIComponent(url.substring(i, j < 0 ? url.length : j));
}

But of course this is still a pretty poor URL parser, but it’s probably okay for something intended to be overridden.

Best would to add some tests. ;)

@alancmclean
Copy link
Author

Yep, this is a lot nicer. Will update.
Introducing tests seems like a separate PR maybe? Got a preference for testing?

@mbostock
Copy link
Owner

That was more a note to myself re. tests. But I’d probably use vows (which is a little long in the tooth but I haven’t bothered to learn a replacement yet) as seen in the smash repo.

@alancmclean
Copy link
Author

Updated with your feedback verbatim.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants