Add support for UNC paths on Windows

[jeremyhaugen]

Resolves #404

[stevearc]

Thanks for the PR! Because this is something I'm not familiar with (windows, and UNC paths in particular) and it's something I can't even test, I'd like to be more rigorous with this area of the code. Could you:

• Check if you need to add anything to the os_to_posix_path method?
• Write up a test plan that covers how you've tested to verify that this works as intended, covering things like
  • creating or deleting files via UNC path
  • moving files from a UNC path to another UNC path
  • moving files from a UNC path to a normal drive path
  • moving files from a normal drive path to a UNC path
• Add some automated tests

[tox2ik]

I'm not the author of the original PR, but I've encountered this issue too and have found another / similar way to solve it.

Add some automated tests

Like if you pass \\foobar\path into the function that it returns //foobar/path?

Please suggest some other tests that would make sense here.
Can you please point us to the place where other relevant, windows-paths things are tested?

My fix looks a little bit different, and handles another bug when navigating to the top of the tree.

---@param path string
---@return string
M.posix_to_os_path = function(path)
  if M.is_windows then
    if vim.startswith(path, "//") then
       -- probably samba share like \\10.3.0.66\foo\bar ,
       local newpath = path:gsub("/", "\\")
       return newpath
    elseif vim.startswith(path, "/") then
      local drive = path:match("^/(%a+)")
      if not drive then
        local newpath = path:gsub("/", "\\")
        return newpath
      end
      local rem = path:sub(drive:len() + 2)
      return string.format("%s:%s", drive, rem:gsub("/", "\\"))
    else
      local newpath = path:gsub("/", "\\")
      return newpath
    end
  else
    return path
  end
end

tested manually:

YES creating or deleting files via UNC path
YES moving files from a UNC path to another UNC path
YES moving files from a UNC path to a normal drive path
YES moving files from a normal drive path to a UNC path
...
YES navigating to the list of drives (or root) from any UNC path.

"YES" means it works with my fix and crashes without the fix.

other bugs found:

when navigating up (the - binding), there is an error saying for example:
Unknown error: \10.3.0.33\

Then navigating up one more time, takes me to the list of all local and mapped drives.
I have only tested my version of the fix where drive == nil is handled

other notes:

this appears to be a result of using telescope find_files. I think it (or rg) returns the paths as \\foo\bar\zumba instead of the mapped drive path M:\zumba (if the drive was mapped like this: net use M \\foo\bar)

original error message:

Error detected while processing BufReadCmd Autocommands for "oil://*":
Error executing lua callback: ...zzo/AppData/Local/nvim-data/lazy/oil.nvim/lua/oil/fs.lua:89: attempt to index local 'drive' (a nil value)
stack traceback:
	...zzo/AppData/Local/nvim-data/lazy/oil.nvim/lua/oil/fs.lua:89: in function 'posix_to_os_path'
	...Local/nvim-data/lazy/oil.nvim/lua/oil/adapters/files.lua:229: in function 'normalize_url'
	...o/AppData/Local/nvim-data/lazy/oil.nvim/lua/oil/init.lua:995: in function 'load_oil_buffer'
	...o/AppData/Local/nvim-data/lazy/oil.nvim/lua/oil/init.lua:1117: in function <...o/AppData/Local/nvim-data/lazy/oil.nvim/lua/oil/init.lua:1116>
Error detected while processing BufEnter Autocommands for "oil:///*":
Error executing lua callback: ...zzo/AppData/Local/nvim-data/lazy/oil.nvim/lua/oil/fs.lua:89: attempt to index local 'drive' (a nil value)
stack traceback:
	...zzo/AppData/Local/nvim-data/lazy/oil.nvim/lua/oil/fs.lua:89: in function 'get_current_dir'
	C:/Users/jazzo/AppData/Local/nvim/lua/genja/fn-oil.lua:37: in function <C:/Users/jazzo/AppData/Local/nvim/lua/genja/fn-oil.lua:35>

In this case J:\notes is pointing to \\10.5.0.3\jaro\notes and the drive was mapped with net use J \\10.5.0.3\jaro

the stuff in lua/genja/fn-oil.lua is my own customization / integration of oil and telescope and should not matter for this bug, although as I stated before this bug shows up only when I use telescope to find / navigate to a new file while current working directory is somewhere in that mapped drive.

Ok, hopefully that is enough context, please advice on how I can proceed with the tests. It would be cool to have this bug fixed in the next version because it breaks my activities on a daily basis :)

I can start a new PR if we don't hear from the original author within a week or two.

[stevearc]

@tox2ik We don't currently have any tests for the utilities in fs.lua, so you would need to add a fs_spec.lua file and put them there. Doesn't have to be much, just cover the basic cases.

when navigating up (the - binding), there is an error saying for example:
Unknown error: \10.3.0.33\

We'll need to figure out what's going on here. There is some special windows handling for when you go up past the current drive:

oil.nvim/lua/oil/adapters/files.lua

Lines 344 to 346 in 42333bb

	if fs.is_windows and path == "/" then
	return list_windows_drives(url, column_defs, cb)
	end

We may need to add some more special handling for these cases as well.

this appears to be a result of using telescope find_files.

What is causing this error? Are you opening oil from telescope, or opening telescope inside of oil?

Since there hasn't been any movement on this PR, feel free to open a new one and continue the conversation there.

[jeremyhaugen]

Thanks for the PR! Because this is something I'm not familiar with (windows, and UNC paths in particular) and it's something I can't even test, I'd like to be more rigorous with this area of the code. Could you:

• Check if you need to add anything to the os_to_posix_path method?

• Write up a test plan that covers how you've tested to verify that this works as intended, covering things like

  • creating or deleting files via UNC path
  • moving files from a UNC path to another UNC path
  • moving files from a UNC path to a normal drive path
  • moving files from a normal drive path to a UNC path

• Add some automated tests

I've added some automated tests in the fs_spec.lua file. I've also manually tested the below scenarios on windows, and all are working fine:

• Creating files on UNC path
• Creating files on local path
• Deleting files on UNC path
• Deleting files on local path
• Moving files between two UNC paths
• Moving files between two local paths
• Moving files from a local path to UNC path
• Moving files from a UNC path to a local path

I have also seen issues when navigating to the root UNC path, as mentioned by tox2ik, but haven't looked into any solution for that.

I added some tests for the "os_to_posix_path" method. I didn't need to make any changes to that for UNC support.

[stevearc]

Fantastic, thanks for testing the behavior! It sounds like there are two issues remaining:

1. When navigating to the root of the share, it produces an error Unknown error: \10.3.0.33\. I'm not entirely sure what's wrong because I don't know where this failure happens.
2. When you navigate up again, it shows all local and mapped drives. Open question: should it also show these network share paths?

[pidgeon777]

Hello, are there any updates on this pull request? This PR seems to address issues with Windows path handling. I've recently encountered a related issue with forward-slash paths on Windows, which I've documented in issue #676: #676. It would be great to see a comprehensive solution for Windows path handling in oil.nvim.