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 open windows path with spaces in prompt #8279

Closed

Conversation

vgwidt
Copy link

@vgwidt vgwidt commented Sep 14, 2023

This allows Windows paths to be opened using the prompt if the path has a space in it (#5921). Right now for :cd or :open, if a path has spaces you must type the entire path manually between quotes. The prompt autocomplete currently puts quotes between folders or files with spaces, but it 1. is not separated properly in shellwords, 2. cannot be opened with quotes in the middle of a path, and 3. puts the separator before the closing quote which breaks the file list builder.

The " characters need to be stripped out from the string if they are in the middle of the path. It is possible to use a path surrounded with doublequotes, and I originally wanted to rewrite the argument to add a " at the beginning of the argument when a file or folder with spaces is selected, but I ended up with too hacky of a solution using shellwords in the change_completion_selection function. I think if there were an ergonomic way to do this, I'd much prefer to do it that way.

I was tempted to add the bit that strips out the quotes in expand_tilde 😅. Not sure if there's a better way to do this.

The caveat with this fix is it still will not work if you start the path with " (i.e. typing :cd "C:"Program Files"" will not work). Entering :cd "C:\Program Files" does work but the autocomplete prompts will not work. I had issues maintainly compatibility with other tests and #4098 when trying to get this to work in shellwords. Maybe it could be worked around by checking if the argument starts with with " but then we run into the same issue above of determining where the argument begins.

@pascalkuthe
Copy link
Member

pascalkuthe commented Sep 14, 2023

Hmm so you seemed to have solved this very generically by allowing arbitrary quoted oath components.

Does windows actually support those (like in a shell). I was under the impression you need to always quite the entire path (which should already work) and the issue here was that auto completions currently don't do that and instead only quote the last component.

@vgwidt
Copy link
Author

vgwidt commented Sep 14, 2023

Hmm so you seemed to have solved this very generically by allowing arbitrary quoted oath components.

Foes windows actually support those (like in a shell). I was under the impression you need to always quite the entire path (which should already work) and the issue here was that auto completions currently don't do that and instead only quote the last component.

The Windows shell does actually support this. It is very forgiving; even cd C:\"Program Files\"Something works. But it is less known, and quoting the whole path is probably preferred as it is more standard.

In #5921 it is mentioned that maybe both ways should be supported, and I guess this PR would at least cover allowing quotes around the individual components. I wonder if there is a way for Rust to handle these paths for Windows without having to explicity remove the quotes...

What could alternatively be done is to remove the automatic inserts of " in the components with spaces as this just doesn't work at all in its current state. Then it would be up to the user to add " at the front if they anticipate using paths with spaces as autocomplete will work properly as long as the components aren't quoted. They don't even have to add a " at the end for it to work :). Then later if there is a more ergonomic way to track the beginning of an argument in the prompt, the " could be automatically added to the beginning when selecting an autocompletion with spaces. I don't think there is a strong need to support individual components being quoted so I'm almost leaning towards this as being the better option now. And the tradeoff with the PR in its current state is that adding a quote at the beginning wouldn't work with autocomplete...

@vgwidt
Copy link
Author

vgwidt commented Jul 13, 2024

Shellwords is being reworked so this can probably be closed.

@vgwidt vgwidt closed this Oct 15, 2024
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