nvim-neo-tree / neo-tree.nvim

Neovim plugin to manage the file system and other tree like structures.
MIT License
3.94k stars 227 forks source link

[Bug] Failed to open NeoTree when currently focused buffer path does not exist #758

Open ray-x opened 1 year ago

ray-x commented 1 year ago

Trying to open neo-tree when active buffer is fugitive git status (:Git command)

image

When I press y to change cwd. A error message popup

image
miversen33 commented 1 year ago

That error is correct. You told it to try and open /fugitive:/home/ray/github/ray-x/go.nvim which is (almost certainly) not a valid directory in your file system (as fugitive gives its buffers names that start with /fugitive:).

Either try without telling it to change the an invalid cwd, or open neotree while the focus is not in a buffer with an invalid file name

ray-x commented 1 year ago

Nop. I did not tell neotree to open /fugitive:xxxxx I was using command NeoTreeShowToggle Also I tried nvim-tree.lua , it does not infer anything of fugitive

Inside the fugitive buffer, the pwd command shows the current path is /home/ray/github/ray-x/go.nvim which is correct. I have no idea why neotree insert fugitive at the front.

Last but not least, If I use 1<C-g> to get the buffer URI, it shows fugitive:///home/ray/github/ray-x/go.nvim which is a correct format according to rfc8089. Please be noted that the buffer can either in the format of URI or old style of file path.

miversen33 commented 1 year ago

Please read the head of the popup in your screenshot.

File not in cwd. Change cwd to /fugitive:/home/ray/github/ray-x/go.nvim?

When I press y to change cwd

You told Neo-tree to open an invalid file location. Granted, the file location probably should reflect fugitive:/// as opposed to /fugitive:/ but that is not really a bug that is worth fixing as both are not valid locations within your filesystem.

As I said

Either try without telling it to change the an invalid cwd, or open neotree while the focus is not in a buffer with an invalid file name

Either press n to tell Neo-tree not infer the cwd from the currently focused buffer name, or open Neo-tree while not focused on a buffer with an invalid path.

ray-x commented 1 year ago

Please refer to rfc-8089. URI fugitive:///home/ray/github/ray-x/go.nvim means the file is in /home/ray/github/ray-x/go.nvim path.

I have no idea why neotree add / in the front. This is URI, not file path. they should be treated differently.

From rfc-8089, the URI format is file://localhost/absolute/path/to/file As the file is a local file, localhost is omitted so it is file:///absolute/path/to/file and in this example, the path to the file is /home/ray/github/ray-x/go.nvim . And the neotree should open that folder. I also checked behavior of Clap filer. It can handle fugitive correctly.

Here is the buffer ls result:

image
cseickel commented 1 year ago

From rfc-8089, the URI format is file://localhost/absolute/path/to/file

But this uri does not start with file:// at all, does it? I realize Fugitive is very popular and maybe on that basis some might think that deserves special handling, but it is still a custom URI scheme and I'm not sure we should be handling the thousands of custom URI schemes we might come across if we make this exception.

To me, the most correct and maintainable thing to do is to recognize that this is not a protocol that is supported and ignore it. The alternative is to add a service or utility function that interprets URIs and decides for each one (fugitive://, 'file://, 'ftp://, 'http://, 'scp://, etc) whether to handle or ignore, and then how to transform the path to the standard representation.

miversen33 commented 1 year ago

A couple things here. 1) The URI (in this case) is not a file URI, its a fugitive URI. Pedantic, but since we are here we might as well be. 1a) Because its a fugitive URI, we should not be treating it as a file URI per RFC 3986

Each URI begins with a scheme name that refers to a specification for assigning identifiers within that scheme. As such, the URI syntax is a federated and extensible naming system wherein each scheme's specification may further restrict the syntax and semantics of identifiers using that scheme. 2) I don't believe it is in the best intention for Neo-tree to attempt to process any and all URI schemes to figure out the correct directory to jump to (and I would guess neither did @cseickel as you are given an explicit popup that asks you if you want to jump to the directory in question) 3) I have no real desire of getting into a theoretical RFC interpretation debate. I will leave that up to the project owner.

I would guess that the reason :pwd works is because fugitive isn't changing the active directory (local or otherwise) its in. Its just opening a buffer with a special URI name.

Could Neo-tree check the output of pwd to get the cwd? Probably. Right now its being interpreted via the buffer name. Related link utils.split_path

-- utils.split_path
---Split string into a table of strings using a separator.
---@param inputString string The string to split.
---@param sep string The separator to use.
---@return table table A table of strings.
M.split = function(inputString, sep)
  local fields = {}

  local pattern = string.format("([^%s]+)", sep)
  local _ = string.gsub(inputString, pattern, function(c)
    fields[#fields + 1] = c
  end)

  return fields
end

Interpreting a URI to maybe get a valid file location seems like a large stretch for this project but I will leave that decision up to @cseickel

cseickel commented 1 year ago

Interpreting a URI to maybe get a valid file location seems like a large stretch for this project but I will leave that decision up to @cseickel

Jinx. We responding at the same time! I think we are on the same page with this one @miversen33. I am always going to be more conservative with regards to edge cases and special situations and prefer to do less unless there is a huge demand or the feature is very isolated.

ray-x commented 1 year ago

1) Fugitive is the most popular git plugin in vim/nvim world 2) This is not a corner case. If you use any plugins open float terminal, you will find the buffer is term:///curret/folder/ , if ssh/sftp to server the file will be ssh://host/folder. Adding a / in the front will never get you the correct path. The neotree will not work in the above cases and prompt misleading path information. 3) I checked nerdtree behavior, it detected the path correctly.

cseickel commented 1 year ago

2. Adding a / in the front will never get you the correct path

Agreed, the problem here is that it is naively assuming that the buffer name is either going to be a terminal or a file. This reminds me that the term:// uri is being handled as a special case: https://github.com/nvim-neo-tree/neo-tree.nvim/blob/74040b34278910d9b467fd914862e2a9a1ebacaa/lua/neo-tree/sources/manager.lua#L153-L155

I think the right thing to do here is to expand that check to be more than just a check for term:// and make it a check for any protocol. I would personally ignore all paths that are not files. Of course, ssh:// and all other protocols are not valid paths either for the filesystem source and should also be ignored. The Netman source could handle those, but that is separate from the filesystem source being discussed here.

If you want to submit a PR that will pick out fugitive:// style paths and transform them appropriately then I will accept it.

miversen33 commented 1 year ago
  1. Fugitive is the most popular git plugin in vim/nvim world

I would argue that this is entirely irrelevant as Neo-tree is not a git plugin. Adding edge case handling for other plugins should not be considered "standard" regardless of how popular that plugin is. That inherently adds cross maintenance of Neo-tree and Fugitive should Fugitive change anything. Now will that plugin change something like this? I mean probably not, but this is more a precedent issue IMO.

  1. I checked nerdtree behavior, it detected the path correctly.

This is likely related to the fact that nerdtree is using getcwd and not parsing the path string itself, as I mentioned here. I don't know that the way we are getting the cwd is correct (I would probably change this to return the output of vim.fn.getcwd() for the cwd param honestly). Nerdtree does not parse the URI

ray-x commented 1 year ago

I do not think it is adding support to other plugins, but it is more neo-tree plugin failing to open when the current buffer is not a file in filesystem. One alternative is using a exclude list similar to nvim-tree to exclude fugitive.

cseickel commented 1 year ago

I would say that this is a more generic problem than just a fugitive buffer, the same effect will happen when you are on a buffer for a file that was deleted and you use a neotree reveal command.

The issue here is that when the buffer has an invalid path, for whatever reason, neo-tree should just ignore it and not throw any errors. If anything at all, it should be a warning.

minusf commented 7 months ago

hello. came here to say something similar:

  1. start editing a remote file: :e scp://wd8/psql-users

  2. Try to open neotree: :Neotree reveal_force_cwd

[Neo-tree ERROR] debounce  filesystem_navigate  error:  vim/_editor.lua:0: nvim_exec2(): Vim(tcd):E344: Can't find directory "/scp:/wd8" in cd path
<press Enter>
[Neo-tree ERROR] /scp:/wd8 :  ENOENT: no such file or directory: /scp:/wd8

and shows an empty Neotree sidebar...

I don't expect neotree to open an ssh session and show me the folder structure on the remote host 😎 it obviously cannot do what it was asked of, but perhaps it could fail more graciously when seeing a netrw style remote URI's and fall back on the actual pwd.

Yes, :Neotree reveal has this popup question safeguard but I don't know if it makes sense for netrw style remote URI's.