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

Fixes for filenames that include spaces #72

Closed
wants to merge 2 commits into from

Conversation

prodonjs
Copy link
Contributor

@prodonjs prodonjs commented Dec 8, 2017

#71
Fix Commands that reference a filename so that the portion of input following the command name and space are considered to be the full path to the file. In the current head, this was being split on the space character and therefore would fail on paths that contained spaces.

@julianhyde
Copy link
Owner

Thanks for this PR. Rather than taking the rest of the line including spaces, I think we should allow quoted file names. For example,

!run "/foo/ bar baz/script with spaces.sql"

I think it would be more consistent with people's expectations; it would also allow us to add more arguments to !run in future. You've already added tests, so if you make this change I will accept your PR.

@prodonjs
Copy link
Contributor Author

prodonjs commented Dec 8, 2017

Added the change to respect quoted filenames. This doesn't handle the generic case of having more arguments to a command like run, but I was running into issues even when passing the properly quoted name of the script through the shell. Java would correct pass the full path in main()'s args, but at that point the quotes would be removed. The change I made in SqlLine addresses that.

@julianhyde
Copy link
Owner

Fixed in c1ac008. Thanks for the PR!

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