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

refactor: use zindex for checking if win is float #139

Merged
merged 2 commits into from
Oct 13, 2023

Conversation

willothy
Copy link
Collaborator

@willothy willothy commented Oct 10, 2023

Relative may soon be delegated to only floats (returning nil for splits), if that's the route I/we decide to take with this PR in core. This PR preemptively addresses that change by checking zindex instead of relative to determine whether a window is floating. This doesn't change any behavior now, so even if that PR isn't merged there will be no side-affects from this, but this ensures we don't break any user configs if/when it is merged.

@cryptomilk
Copy link
Collaborator

Can you please add a test? You can probably use screenshot testing for this. Example: https://github.com/echasnovski/mini.nvim/blob/main/tests/test_jump2d.lua#L433

If you set force, then a screenshot will be created, once you have one you can remove it, see: https://github.com/echasnovski/mini.test/blob/main/doc/mini-test.txt#L568

@willothy
Copy link
Collaborator Author

Will do - can't I just check whether the window is floating in the test though? I'm a bit confused on what screenshot would be used to check here

@cryptomilk
Copy link
Collaborator

Well, it should not resize the float. So a screenshot will check the float is still intact.

@willothy
Copy link
Collaborator Author

Well, it should not resize the float. So a screenshot will check the float is still intact.

Gotcha, sounds good :)

@willothy
Copy link
Collaborator Author

win_get_width and win_get_height should be enough for this I think, not sure screenshot is needed.

@cryptomilk
Copy link
Collaborator

Looks fine for me. Thanks!

@cryptomilk cryptomilk merged commit 5269ea7 into nvim-focus:master Oct 13, 2023
6 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