Skip to content

Conversation

@davisthedev
Copy link
Contributor

@davisthedev davisthedev commented Aug 14, 2023

fixes #2352

  • add check if environment is windows
  • if windows, escape special characters

This works on windows, will need additional linux/mac testing.

@alex-courtis alex-courtis changed the title Fix escape special characters on windows fix(#2374): escape special characters on windows on edit Aug 14, 2023
@alex-courtis
Copy link
Member

Looks good, behind a feature flag.

Do we need to do the same for other actions such as node.open.replace_tree_buffer or node.open.tab?

We might also be able to enhance utils.canonical_path to achieve this fix.

@gegoune
Copy link
Collaborator

gegoune commented Aug 14, 2023

We definitely should have dedicated function to do that

@davisthedev
Copy link
Contributor Author

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.
That is why I only made the change at the last point.

@alex-courtis
Copy link
Member

I am happy to add this in other areas or let someone with more knowledge handle it and I can test.

I am confident it your abilities.

I am not sure if there would be any adverse effects to other pieces of code with the escaping. That is why I only made the change at the last point.

We can't cover every possible case, however we can:

  1. Test and maybe fix the different actions that edit e.g. open in tab, open in place etc.
  2. Test all codepaths that lead to this common function.

This is behind the feature flag so we can be confident.

@davisthedev
Copy link
Contributor Author

I did not have any issues with node.open.replace_tree_buffer or node.open.tab.
I did move the code to its own utils function so it can be used if the problem is found elswhere.

@alex-courtis alex-courtis changed the title fix(#2374): escape special characters on windows on edit fix(#2352): windows: escape special characters on edit Aug 16, 2023
@alex-courtis alex-courtis changed the title fix(#2352): windows: escape special characters on edit fix(#2352): windows: escape special filename characters on edit Aug 16, 2023
Copy link
Member

@alex-courtis alex-courtis left a 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.

@davisthedev
Copy link
Contributor Author

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.

@alex-courtis
Copy link
Member

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.

Copy link
Member

@alex-courtis alex-courtis left a 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

@alex-courtis alex-courtis merged commit 7c4c7e4 into nvim-tree:master Aug 20, 2023
@xarthurx
Copy link

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.

neovim/neovim#24542

I don't expect they will change it in the near future.

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.

Cannot open file whose filename or folder name has parenthesis "(" on Windows

4 participants