fix(#2961): windows: escape brackets and parentheses when opening file #2962

ljie-PI · 2024-10-19T06:41:34Z

Description
With this change (45a93d9#diff-eeb21711fceb0abba1450bd1adcbecc4fd90cf8ce4d5d7a8233f886c1bebd41fR375), I found it doesn't work well on Windows for 2 cases:

When I open a file from nvim-tree, the root of nvim-tree window changed to the directory of that file
The buffer of this file can not be listed when I use telescope command <cmd>lua require('telescope.builtin').buffers(require('telescope.themes').get_dropdown{previewer = false})<cr> Neovim version

This PR is to revert the change and fix the issue #2961 and #2862

…ashes (#2903)" This reverts commit 45a93d9.

alex-courtis · 2024-10-20T03:32:45Z

lua/nvim-tree/utils.lua

@@ -59,6 +59,17 @@ function M.path_basename(path)
  return path:sub(i + 1, #path)
 end

+--- Check if there are parentheses before brackets, it causes problems for windows.
+--- Refer to issue #2862 and #2961 for more details.
+local function has_parentheses_and_brackets(path)


Great work fixing this one.

alex-courtis · 2024-10-20T03:33:46Z

lua/nvim-tree/utils.lua

 --- Escapes special characters in string if windows else returns unmodified string.
 ---@param path string
 ---@return string|nil
 function M.escape_special_chars(path)
  if path == nil then
    return path
  end
-  return M.is_windows and path:gsub("\\", "/") or path
+  return M.is_windows and escape_special_char_for_windows(path) or path


Thank you, this one definitely should be behind the windows feature flag.

alex-courtis

Many thanks for this fix! This all looks reasonable and I can see how you have tested comprehensively.

The is_windows feature flag only applies to powershell, hence you do not need to test WSL.

The nvim-tree developers don't have access to nor expertise with windows, if there is an issue we expect you to resolve it.

All windows fixes must go behind the feature flags utils.is_windows etc. The existing codepaths for non-windows must remain unchanged.

alex-courtis · 2024-10-20T03:37:56Z

lua/nvim-tree/actions/node/open-file.lua

@@ -370,36 +370,35 @@ end
 ---@param mode string
 ---@param filename string
 function M.fn(mode, filename)
-  local fname = utils.escape_special_chars(filename)


This is a change in behaviour that will affect linux and macos users.

Please put this behind the windows feature flag, to ensure that it only applies to windows users.

Something like

local fname if utils.is_windows then fname = filename else fname = utils.escape_special_chars(filename) end

Hi @alex-courtis , change of this line is caused by the revert of previous fix. I think there wasn't such line at the begining.

Please accept my apologies, I did not Seek First To Understand.

git diff bd4881660bf0ddfa6acb21259f856ba3dcb26a93 lua/nvim-tree/actions/node/open-file.lua

shows that the change #2903 has been completely reverted.

Yes, I reverted all changes of #2903 .

In #2903 util.escape_special_chats is called here and impact all file names below, which are not correct. (my tests failed)

Now and before #2903 utils.escape_special_chars is called in the open_in_new_window function (line 334 and 336), so I think I'm just following the logic before #2903 . And the is_windows flag is already in the utils.escape_special_chars function, so not added outside utils.escape_special_chars

alex-courtis

Looking good, just the one windows feature flag check needed.

Alternative idea:
I was able to cleanly git revert 45a93d99794fff3064141d5b3a50db98ce352697 on master
You could instead do that on a new branch and apply your fixes on top.
That would result in cleaner history and a simpler PR.
Your choice.

alex-courtis · 2024-10-20T21:51:05Z

lua/nvim-tree/actions/node/open-file.lua

@@ -370,36 +370,35 @@ end
 ---@param mode string
 ---@param filename string
 function M.fn(mode, filename)
-  local fname = utils.escape_special_chars(filename)


Please accept my apologies, I did not Seek First To Understand.

git diff bd4881660bf0ddfa6acb21259f856ba3dcb26a93 lua/nvim-tree/actions/node/open-file.lua

shows that the change #2903 has been completely reverted.

alex-courtis · 2024-10-20T22:05:46Z

lua/nvim-tree/utils.lua

+  end
+
+  local _, r = norm_path:find(M.path_add_trailing(relative_to), 1, true)
+  local p = norm_path


This path is executed for all OS, which may cause issues for such paths. Please execute only when is_windows

This injected error is definitely thrown on linux, showing that the code path is executed:

--- Check if there are parentheses before brackets, it causes problems for windows. --- Refer to issue #2862 and #2961 for more details. local function has_parentheses_and_brackets(path) error("has_parentheses_and_brackets")

E5108: Error executing lua: ...d/site/pack/packer/start/ljie-PI/lua/nvim-tree/utils.lua:65: has_parentheses_and_brackets called stack traceback: [C]: in function 'error' ...d/site/pack/packer/start/ljie-PI/lua/nvim-tree/utils.lua:65: in function 'has_parentheses_and_brackets' ...d/site/pack/packer/start/ljie-PI/lua/nvim-tree/utils.lua:84: in function 'path_relative' .../packer/start/ljie-PI/lua/nvim-tree/explorer/filters.lua:161: in function 'custom' .../packer/start/ljie-PI/lua/nvim-tree/explorer/filters.lua:247: in function 'should_filter_as_reason'

Added flag on line 84, so that norm_path will keep unchanged for non-windows OS.

This reverts commit ada77cb.

alex-courtis · 2024-10-21T21:34:59Z

Thank you I'll get to this review on the weekend.

…

On Mon, 21 Oct 2024, 18:34 Jie Liu, ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In lua/nvim-tree/actions/node/open-file.lua <#2962 (comment)> : > @@ -370,36 +370,35 @@ end ***@***.*** mode string ***@***.*** filename string function M.fn(mode, filename) - local fname = utils.escape_special_chars(filename) Yes, I reverted all changes of #2903 <#2903> . In #2903 <#2903> util.escape_special_chats is called here and impact all file names below, which are not correct. (my tests failed) Now and before #2903 <#2903> utils.escape_special_chars is called in the open_in_new_window function (line 334 and 336), so I think I'm just following the logic before #2903 <#2903> . And the is_windows flag is already in the utils.escape_special_chars function, so not added outside utils.escape_special_chars — Reply to this email directly, view it on GitHub <#2962 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AALPQYQZ23BHI6M3FTF6BETZ4SVABAVCNFSM6AAAAABQHGGFVWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDGOBRGE3TKMZWGU> . You are receiving this because you were mentioned.Message ID: ***@***.***>

alex-courtis

Many thanks for your contribution!

Revert "fix(#2862): windows path replaces backslashes with forward sl…

25d4f14

…ashes (#2903)" This reverts commit 45a93d9.

ljie-PI mentioned this pull request Oct 19, 2024

[On windows] Root of nvim-tree changed unexpectedly and buffer not listed by telescope #2961

Closed

ljie-PI added 3 commits October 19, 2024 17:18

fix the case when '()' and '[]' are both in file path

cbcadc6

remove debug messages

952e0fd

remove unnecessary comments

9176eab

alex-courtis reviewed Oct 20, 2024

View reviewed changes

alex-courtis requested changes Oct 20, 2024

View reviewed changes

alex-courtis changed the title ~~fix(#2961): [On windows] Root of nvim-tree changed unexpectedly and buffer not listed by telescope~~ fix(#2961): windows: escape brackets and parentheses when opening file Oct 20, 2024

ljie-PI added 3 commits October 21, 2024 15:08

add is_windows feature flag when normalizing path

2b110a6

add is_windows flag for filename change

ada77cb

Revert "add is_windows flag for filename change"

dd80ce0

This reverts commit ada77cb.

alex-courtis approved these changes Oct 24, 2024

View reviewed changes

Merge branch 'master' into win_root_change

7ddae1d

alex-courtis merged commit 63c7ad9 into nvim-tree:master Oct 25, 2024
5 checks passed

github-actions bot mentioned this pull request Oct 25, 2024

chore(master): release nvim-tree 1.8.0 #2943

Merged

ljie-PI deleted the win_root_change branch October 25, 2024 16:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(#2961): windows: escape brackets and parentheses when opening file #2962

fix(#2961): windows: escape brackets and parentheses when opening file #2962

ljie-PI commented Oct 19, 2024 •

edited

Loading

alex-courtis Oct 20, 2024 •

edited

Loading

alex-courtis Oct 20, 2024

alex-courtis left a comment

alex-courtis Oct 20, 2024

ljie-PI Oct 20, 2024

alex-courtis Oct 20, 2024

ljie-PI Oct 21, 2024

alex-courtis left a comment

alex-courtis Oct 20, 2024

alex-courtis Oct 20, 2024

ljie-PI Oct 21, 2024

alex-courtis commented Oct 21, 2024 via email

alex-courtis left a comment

fix(#2961): windows: escape brackets and parentheses when opening file #2962

fix(#2961): windows: escape brackets and parentheses when opening file #2962

Conversation

ljie-PI commented Oct 19, 2024 • edited Loading

alex-courtis Oct 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alex-courtis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alex-courtis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alex-courtis commented Oct 21, 2024 via email

alex-courtis left a comment

Choose a reason for hiding this comment

ljie-PI commented Oct 19, 2024 •

edited

Loading

alex-courtis Oct 20, 2024 •

edited

Loading