-
-
Notifications
You must be signed in to change notification settings - Fork 625
fix(#2352): windows: escape special filename characters on edit #2374
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(#2352): windows: escape special filename characters on edit #2374
Conversation
Looks good, behind a feature flag. Do we need to do the same for other actions such as We might also be able to enhance |
We definitely should have dedicated function to do that |
I am happy to add this in other areas or let someone with more knowledge handle it and I can test. I am not sure if there would be any adverse effects to other pieces of code with the escaping. |
I am confident it your abilities.
We can't cover every possible case, however we can:
This is behind the feature flag so we can be confident. |
I did not have any issues with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Many thanks for your contribution. I'll get back to this one on the weekend.
I was tempted to put this everywhere we use a fnameescape
however it's safest just to fix known issues.
I don't know if the change even needs to go everywhere fnameescape is used. The issue only seems to happen whenever the path is handed over to neovim to create a buffer. It seems like everything is fine inside nvim-tree. I am working on a similar merge for telescope, and it has the same thing. It only has this problem when it tries to open the file in a buffer or window. So I think the root of might actually be fixable inside neovim, but I am not ready to work on that codebase yet. |
That sounds quite reasonable. We are escaping paths using vim's api, it's just these special cases that are not correct. We might encounter an issue if this is fixed in nvim, however we cannot do anything until that time. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Many thanks for your contribution
I don't expect they will change it in the near future. |
fixes #2352
This works on windows, will need additional linux/mac testing.