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

feat: Add ability to hijack-cursor #955

Merged
merged 11 commits into from Jan 21, 2024

Conversation

ghostbuster91
Copy link
Contributor

@ghostbuster91 ghostbuster91 commented May 27, 2023

This closes #604

TODO:

  • docs
  • config
  • proper initialization

@ghostbuster91
Copy link
Contributor Author

@cseickel Thanks for the quick review. I have addressed your comments. Is there anything left that I should do? Initially I was thinking also about mentioning it in the vim docs, but I see that not all of the flags are there, so I am not sure if I should put it there.

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.

As far as docs go, not everything is in the help file because I was just too lazy to write docs for every single option. The general rule is that anything that can easily be explained with an inline comment in the config doesn't need to be explicitly listed in the help file.

If you wanted to put in a doc entry, you could just start a new section that could someday contain all of the options that just need one-line explanations like this one. Or you can skip it.

lua/neo-tree/sources/common/hijack_cursor.lua Outdated Show resolved Hide resolved
lua/neo-tree/sources/common/hijack_cursor.lua Outdated Show resolved Hide resolved
@ghostbuster91
Copy link
Contributor Author

Hi @cseickel, it seems that I can now get back to this one, though I still don't know where to subscribe for the events. Could you drop some hint? 🙏

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.

I'm not sure why, but it does not seem to work with position = "current".

if vim.o.filetype ~= "neo-tree" then
return
end
local source = vim.api.nvim_buf_get_var(0, "neo_tree_source")
Copy link
Contributor

Choose a reason for hiding this comment

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

I would make this a pcall because it will throw an error if the variable is not set. Most likely that would only happen from a timing problem.

The important thing is that it would get real annoying if something did cause an error on every cursor moved event.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, done.

lua/neo-tree/sources/common/hijack_cursor.lua Outdated Show resolved Hide resolved
@ghostbuster91
Copy link
Contributor Author

I'm not sure why, but it does not seem to work with position = "current".

I will look into that.

@ghostbuster91
Copy link
Contributor Author

@cseickel I looked into that. It doesn't work because state.winid is nil. I thought that maybe this is some special case that needs to be handled.
I found this https://github.com/ghostbuster91/neo-tree.nvim/blob/11f6042509d8e4dc3ffbddf0bf1d8b7d01ac3d94/lua/neo-tree/ui/selector.lua#L254 and made a similar change. However after that change neotree failed because state.tree is also nil 👀 (in this line https://github.com/ghostbuster91/neo-tree.nvim/blob/11f6042509d8e4dc3ffbddf0bf1d8b7d01ac3d94/lua/neo-tree/sources/common/hijack_cursor.lua#L21)
Any idea what is happening here?

@cseickel
Copy link
Contributor

cseickel commented Jul 26, 2023

Any idea what is happening here?

Yes. The problem is that if you call manager.get_state() without specifying a window id, you will get back the sidebar state for the tab. What you need to do is first check if the window was open in position = "current" or not, and if the position is "current" then you need to pass the winid as the third parameter to get_state(source, tabid, winid) to get the state for that window and not the tab.

You'd need something like this:

local winid = nil
local _, position = pcall(vim.api.nvim_buf_get_var, 0, "neo_tree_position")
if position == "current" then
  winid = vim.api.get_current_win()
end
local state = manager.get_state(source, nil, winid)

That block of logic should probably be a utility function, or maybe the get_state function itself should be checking if the currently focused window is a neo-tree window and doing that work for you.

@ghostbuster91
Copy link
Contributor Author

I see, thanks for the explanation. For now I will just add this check here but I think that it would make sense to move it eventually to get_state. I can do that as a follow-up.

@cseickel
Copy link
Contributor

Hey @ghostbuster91, sorry I think I dropped the ball on this. Is this good to go?

@ghostbuster91
Copy link
Contributor Author

@cseickel Sorry, this is totally my fault. I used this ticket to get some hands-on experience with neotree codebase as this ticket seemed quite simple. However obstacles that I encountered caused my motivation to go away.

Having said that, I think that it would be a waste if I didn't finish it, so I will try to get that last mile done.

@ghostbuster91
Copy link
Contributor Author

@cseickel I fixed the issue with position=current. Could you give it a try?

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 and it works for me in all situations. Thanks!

@cseickel cseickel merged commit e578fe7 into nvim-neo-tree:main Jan 21, 2024
2 checks passed
@ghostbuster91 ghostbuster91 deleted the hijack-cursor branch January 21, 2024 18:14
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.

[Feature request] nvim-tree hijack_cursor functionality
2 participants