mikavilpas / yazi.nvim

A Neovim Plugin for the yazi terminal file manager
MIT License
216 stars 7 forks source link

fix: crash when current file contains "()" characters in its path/name #76

Closed mikavilpas closed 1 month ago

mikavilpas commented 1 month ago

Fixes https://github.com/mikavilpas/yazi.nvim/issues/75

Recently we have had some issues with files that contain special characters in their path / name. Some issues are also specific to Windows which I don't currently have a way to test at all.

For this reason, I would like to summon some people to test this 😄

@vargasd @Kayzels

In lazy.nvim, you can do so by adding a branch instruction to your plugin spec. Then you need to update the plugin using e.g. :Lazy.

Here's my config as a starting point:

{
    "mikavilpas/yazi.nvim",
    branch = "fix-paths-with-()-characters",
    dependencies = {
      "nvim-lua/plenary.nvim",
    },
    event = "VeryLazy",
    keys = {
      {
        "<up>",
        function()
          require("yazi").yazi()
        end,
        desc = "Open the file manager",
      },
      {
        "<s-up>",
        function()
          require("yazi").yazi(nil, vim.fn.getcwd())
        end,
        desc = "Open the file manager in the cwd",
      },
    },
    ---@type YaziConfig
    opts = {
      open_for_directories = true,
      enable_mouse_support = true,
      -- log_level = vim.log.levels.DEBUG,
    },
  }
Kayzels commented 1 month ago

Unfortunately, it immediately breaks on Windows:

error: unexpected argument: 'C:\Users\Kyzan\AppData\Local\nvim\lua\plugins\editor.lua' found

Seems to be doubling the quotation marks now, log file states:

[2024-05-17 11:41:30] DEBUG Opening yazi with the command: (yazi ""C:\Users\Kyzan\AppData\Local\nvim\lua\plugins\editor.lua"" --local-events "rename,delete,trash,move" --chooser-file "C:\Users\Kyzan\AppData\Local\Temp\nvim.0\DXRKoF\0" > "C:\Users\Kyzan\AppData\Local\Temp\nvim.0\DXRKoF\1")

vim.fn.shellescape already adds the double quotes around the string, so with this change, the Windows check to add the quotes should no longer be needed.

Kayzels commented 1 month ago

I haven't tested it with one with () yet, because it will crash for any path given in at the moment.

vargasd commented 1 month ago

It does work on macOS now at least.

mikavilpas commented 1 month ago

Ok, thanks for the feedback. I now reverted to using only vim.fn.shellescape. Can you try it out on windows now?

Long term, I would like there to be a way to test future changes out on Windows #77

Kayzels commented 1 month ago

Just tested it now, and it works perfectly. Definitely an easier solution than what was there previously.

I agree that testing it for Windows would be a good idea. Unfortunately, I have no experience with CI, otherwise I'd help out with that.

mikavilpas commented 1 month ago

Awesome! Thanks for the help.