Skip to content

Conversation

@tkelman
Copy link
Contributor

@tkelman tkelman commented Jun 10, 2016

Backslashes aren't valid in urls, and this happens to also fix the outside-of-base
code below that uses libgit2 since LibGit2.path always uses forward slashes
(behavior inherited from the underlying git_repository_path function)

wip until I write some tests to cover the cases this fixes

@tkelman tkelman added the system:windows Affects only Windows label Jun 10, 2016
@tkelman tkelman added the needs tests Unit tests are required for this change label Jun 10, 2016
@tkelman tkelman changed the title Replace '\\' with '/' for Windows filenames in Base.url WIP: Replace '\\' with '/' for Windows filenames in Base.url Jun 10, 2016
@tkelman tkelman removed the needs tests Unit tests are required for this change label Jun 10, 2016
@tkelman tkelman changed the title WIP: Replace '\\' with '/' for Windows filenames in Base.url Replace '\\' with '/' for Windows filenames in Base.url Jun 10, 2016
@tkelman tkelman force-pushed the tk/urlbackslash branch 3 times, most recently from a14f2d4 to 228f5fe Compare June 12, 2016 21:09
tkelman added 2 commits June 12, 2016 14:13
Backslashes aren't valid urls, and this happens to also fix the outside-of-base
code below that uses libgit2 since `LibGit2.path` always uses forward slashes
(behavior inherited from the underlying `git_repository_path` function)

Temporary directories used by tests are inside a symlink on mac,
so use realpath in startswith check
@tkelman
Copy link
Contributor Author

tkelman commented Jun 12, 2016

Apparently OS X temporary directories are under a symlink, so thanks to Travis I found and fixed a third bug in this same bit of code.

@JeffBezanson
Copy link
Member

Can this be merged now?

@tkelman tkelman merged commit b848fd8 into master Jun 14, 2016
@tkelman tkelman deleted the tk/urlbackslash branch June 14, 2016 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

system:windows Affects only Windows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants