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: skip normalizing empty file paths #1363

Merged
merged 2 commits into from Feb 25, 2024
Merged

Conversation

bwpge
Copy link
Contributor

@bwpge bwpge commented Feb 25, 2024

This PR fixes a quite niche Windows bug where quickly changing popup windows could cause Neo-tree to try and follow (and thus normalize) an nil file path. This causes an error message to be printed, despite not actually being an issue for Neo-tree.

Essentially the problem is:

  • Some form of popup window (like NuiInput) closes, changing the active buffer
  • This causes the "follow" event to be fired from Neo-tree, but it is debounced:
    utils.debounce("neo-tree-follow", function()
  • A toggleterm floating window is opened (triggered from the previous NuiInput by on_submit) and changes the active buffer to be a term:// path before the debounce timer finishes
  • The follow_internal function does not include term:// paths when calling manager.get_path_to_reveal() so it returns nil and passes that directly into normalize_path:
    local path_to_reveal = utils.normalize_path(manager.get_path_to_reveal())
  • On Windows, this path value is indexed here, but the function does not check for nil (note the doc comment states the argument is a string not string?):
    path = path:sub(1, 1):upper() .. path:sub(2)
  • Thus an error occurs before the follow_internal can gracefully exit early from an empty path here:
    if not utils.truthy(path_to_reveal) then

The check is only added after the M.is_windows branch, so this will not change behavior for any non-Windows users. Windows users will have the exact same behavior (no file will be followed), just without the error message.

See below comment.

@bwpge
Copy link
Contributor Author

bwpge commented Feb 25, 2024

After looking at this again, I realized there was a more correct fix, e.g., not passing nil to a function that only accepts string arguments. This fixes the same problem, but doesn't add an extra branch check to every Windows call to normalize_path.

Copy link
Contributor

@cseickel cseickel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thanks for the fix!

@cseickel cseickel merged commit 459c603 into nvim-neo-tree:main Feb 25, 2024
4 checks passed
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.

None yet

2 participants