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

Use unescape for paths before reading files #188

Merged
merged 3 commits into from Nov 27, 2014

Conversation

RST-J
Copy link
Contributor

@RST-J RST-J commented Nov 25, 2014

This re-fixes #100 (it was fixed until the last commit in #174 introduced convert_path).

@hoxworth
Copy link
Contributor

👍

@iainbeeston
Copy link
Contributor

Would it be easy to add a test for that so it doesn't break again?

Otherwise this looks good to merge.

@RST-J
Copy link
Contributor Author

RST-J commented Nov 26, 2014

I'll have a look at it.

@RST-J
Copy link
Contributor Author

RST-J commented Nov 27, 2014

I found a spot where we didn't use convert_path yet. Because of this, the tests in test/test_uri_related.rb that have the ref schema path with spaces didn't fail.
I one removes unescape in custom_open now, these will also fail in a checkout directory that does not contain spaces.
If that still not suffices we have to come up with a test that passes in a file name with spaces directly.

@iainbeeston
Copy link
Contributor

If you think it's not too hard to replicate, then I think it'd be good to add a test for it, but if it's hard, then don't bother (and merge this!)

@iainbeeston
Copy link
Contributor

The again, the build is failing

@RST-J
Copy link
Contributor Author

RST-J commented Nov 27, 2014

Strange, on my machine, using Ruby 2.0.0, the tests all pass.

@RST-J
Copy link
Contributor Author

RST-J commented Nov 27, 2014

Now it worked without that I've changed anything with respect to the previously failing test. Strange, maybe execution order plays a role here.

RST-J added a commit that referenced this pull request Nov 27, 2014
Use unescape for paths before reading files
@RST-J RST-J merged commit b690828 into voxpupuli:master Nov 27, 2014
@RST-J RST-J deleted the unescape_file_paths branch November 28, 2014 00:11
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.

When extending a schema in folder names with spaces
3 participants