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(#2733): escape trash path #2735

Merged
merged 5 commits into from
Apr 6, 2024
Merged

fix(#2733): escape trash path #2735

merged 5 commits into from
Apr 6, 2024

Conversation

alex-courtis
Copy link
Member

fixes #2733

@gegoune
Copy link
Collaborator

gegoune commented Mar 31, 2024

There were some changes to paths handling on Neovim master recently that might be relevant here, but I didn't follow them up very closely, just FYI.

@alex-courtis
Copy link
Member Author

There were some changes to paths handling on Neovim master recently that might be relevant here, but I didn't follow them up very closely, just FYI.

Any hints as to what I could look for?

Rather than using fnameescape directly we could have vim.fn.jobstart escape it for us, something like this:

    local job = vim.fn.jobstart({ "gio", "trash", node.absolute_path }, {

That's how we escape paths for vim.fn.system, vim.loop.spawn etc.

Unfortunately we would need to find a way to split config.trash.cmd into a list.

@gegoune
Copy link
Collaborator

gegoune commented Apr 1, 2024

Any hints as to what I could look for?

I think those are the two that I meant:
neovim/neovim#28105
neovim/neovim#28064

@alex-courtis
Copy link
Member Author

I think those are the two that I meant: neovim/neovim#28105 neovim/neovim#28064

Today I learned about vim.fs. Guessing it's recently added.

We're not using it but should probably migrate some of our fnamemodify calls and utils like path_basename

@alex-courtis
Copy link
Member Author

On further review it seems we should use the fit for purpose shellescape rather than fnameescape.

@alex-courtis alex-courtis merged commit 81eb8d5 into master Apr 6, 2024
5 checks passed
@alex-courtis alex-courtis deleted the 2733-escape-trash-path branch April 6, 2024 01:28
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.

Parent folder deleted when deleting a folder starting with $
2 participants