Skip to content

Conversation

@tomv564
Copy link
Contributor

@tomv564 tomv564 commented Aug 7, 2017

Fix for #85 where windows paths were not prefixed and converted to forward slashes

Copy link
Contributor

@gatesn gatesn left a comment

Choose a reason for hiding this comment

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

Good catches here. One thing I'd say, is do we need to url quote the path we use in the uri_like function? I know it's not part of this Pr but may as well slip it in

@tomv564
Copy link
Contributor Author

tomv564 commented Aug 7, 2017

Sounds like a good idea, I'll add quoting.
It will try to (unnecessarily) encode the : in /D:/myfile which I'd like to avoid.
I couldn't find any reason why module_path is sometimes None, please let me know if my assumption was correct!

unicode objects.
"""
parts = list(urlparse(doc_uri))
if path[0] != '/': # fix path for windows
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add and ':' in path just to make sure we don't throw on the next line

Copy link
Contributor

@gatesn gatesn left a comment

Choose a reason for hiding this comment

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

LGTM

@gatesn gatesn merged commit a78885d into palantir:develop Aug 7, 2017
@tomv564 tomv564 deleted the fix_uri_like_windows branch October 24, 2017 19:38
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