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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: set correct winid when window.position = current #874

Merged
merged 5 commits into from
Apr 18, 2023

Conversation

benplante
Copy link
Contributor

Fixes #845 and #822

:Neotree and the default hijack_netrw behaviour would fail if config.window.postion = current. It seems that both execute incommand/init.lua and hijack in setup/netrw.lua didn't set the correct winid in state when config.window.position = current. Changes to fix this are minimal and just involve getting the window position from the config.

This is my first PR to open source 馃コ and I'm quite new to neovim plugin development, any feedback would be appreciated.

@cseickel cseickel changed the base branch from v2.x to main April 15, 2023 22:08
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.

Thanks for contributing! The only issue, which I have fixed for you, is that all PRs should be targeting the main branch instead of the release branch. Other than that, there is just one comment.

lua/neo-tree/setup/netrw.lua Outdated Show resolved Hide resolved
@@ -5,14 +5,15 @@ local command = require("neo-tree.command")
local M = {}

local get_position = function(source_name)
local pos = utils.get_value(M, "config." .. source_name .. ".window.position", "left")
local nt = require("neo-tree")
local pos = utils.get_value(nt.config, source_name .. ".window.position", default_pos, true)
Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't actually have any effect with how this is used right now, but I think the default_pos variable is undefined here and should be replaced with "left".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, fixed that now.

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!

@cseickel cseickel merged commit a4733cd into nvim-neo-tree:main Apr 18, 2023
2 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.

Neotree setting position=current is broken
2 participants