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

fix broken path in chat example in windows #4648

Closed
wants to merge 12 commits into from

Conversation

disizali
Copy link
Contributor

@disizali disizali commented Apr 6, 2020

fix this : #4647

@CLAassistant
Copy link

CLAassistant commented Apr 6, 2020

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@Drunpy Drunpy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the interest in contributing!

I don't think it is the best approach right now since "./index.html" is already declared at const u = ...
also u.pathname should return the same "/index.html".

It's important to note that in Ubuntu 18.04 it's running without problems

@disizali disizali changed the title fix broken path in chat example fix broken path in chat example in windows Apr 6, 2020
@disizali
Copy link
Contributor Author

disizali commented Apr 6, 2020

Thank you very much and I hope to be able to collaborate on the project in the long run

Unfortunately, this is not the case in Windows, and u.pathname is not same to "./index.html" , it will be :
/C:/Users/Ali/Desktop/deno/std/examples/chat/index.html

@Drunpy
Copy link
Contributor

Drunpy commented Apr 6, 2020

I see, as you mentioned in #4647 the problem is the same in node. This seems to be within URL parsing rather than in server.ts. I've found this issue with a deeper troubleshooting at this topic, you may drive your solution there. I'm willing to help, just hit me up.

@disizali
Copy link
Contributor Author

disizali commented Apr 6, 2020

new URL href & pathname are standardized [here] and cannot be changed separately in node or deno ,
problem is first slash in given pathname

in node , a function in url library [here] is provided for convert this kind of file path in windows , which solves this problem .

anyway , currently this file has problems on Windows, and the shortest way to solve is to use ./index.html

alternatives :
in deno fs module itself used this function [here] :

function urlToFilePath(url: URL): string {
  // Since `new URL('file:///C:/a').pathname` is `/C:/a`, remove leading slash.
  return url.pathname.slice(url.protocol == "file:" && isWindows ? 1 : 0);
}

or another approach in node module [ here ]:

and the last solution is to change fs readFile functions behavior to work with file:/// path entirely.

@Drunpy
Copy link
Contributor

Drunpy commented Apr 6, 2020

I would go with node's approach. Simple because it's already done there and we know it works.

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.

3 participants