mrjones2014 / smart-splits.nvim

๐Ÿง  Smart, seamless, directional navigation and resizing of Neovim + terminal multiplexer splits. Supports tmux, Wezterm, and Kitty. Think about splits in terms of "up/down/left/right".
MIT License
977 stars 43 forks source link

Incorrect resizing with more complicated layous #11

Closed mehalter closed 2 years ago

mehalter commented 2 years ago

Great work on this plugin! I am testing it out to fix my split resizing woes. I just wanted to report that for somewhat complicated split layouts the resizing bindings stop working as I would expect them to. This layout is an example:

2022-03-23_10:17:31_screenshot

This is 5 splits in the following order: vertical split, horizontal split on the right, and a horizontal split on the left to make a 2x2 grid. Then finally doing a vertical split on the top left box in the grid.

After that is created if I go to the split that is highlighted in the screenshot, resize right moves the left edge to the left and resize left moves the left edge to the right. It looks like this happens any time I have nested splits of different directions.

mrjones2014 commented 2 years ago

Interesting. I'm able to reproduce, not sure what is causing it though. Will look into this. Thanks for reporting!

mrjones2014 commented 2 years ago

I think it's due to Neovim having slightly inconsistent behavior with how it resizes in complex layouts like this. I may be able to work out a solution by detecting certain scenarios.

It seems to only happen when you're in a window that:

So I might be able to do something like

mrjones2014 commented 2 years ago

actually that heuristic won't work either because this layout does already work correctly

CleanShot-2022-03-23-at-11 20 32

mrjones2014 commented 2 years ago

this is gonna be tough to solve lol

mrjones2014 commented 2 years ago

I think it's due to Neovim having slightly inconsistent behavior with how it resizes in complex layouts like this. I may be able to work out a solution by detecting certain scenarios.

It seems to only happen when you're in a window that:

  • is not at the left or right edge (it's middle of >2 windows)
  • Has a horizontal split below it that spans across multiple windows

So I might be able to do something like

  • if middle window
  • and if there's a window below
  • and if the window below is >= the width of current window plus window to the left
  • then reverse resize direction

I think maybe this but:

mehalter commented 2 years ago

@mrjones2014 this issue seems to only arise when I make a horizontal split after a vertical split. (I think that is the pattern here)

Try again to make a grid first with a vertical split and then a horizontal split the right window and try to resize the bottom right box. It is backwards. I think that is the most simple example I can find and also a split that I use a lot 2022-03-23_11:27:09_screenshot

mehalter commented 2 years ago

Also in this example the issue is highlighted in a non middle grid box and does not have a horizontal split below it that spans across multiple windows

mrjones2014 commented 2 years ago

Oh, interesting. You're totally right. All it takes is making a horizontal split after a vertical split.

mrjones2014 commented 2 years ago

No idea what the heck would cause that though : |

mrjones2014 commented 2 years ago

I think this is why. In this recording, I am only going once left, then once right (moving cursor between splits). Since going back right doesn't return to the previous split, it identifies the original split as a middle split when it should be an edge split.

CleanShot-2022-03-23-at-11 32 29

mrjones2014 commented 2 years ago

Not sure the best way to resolve that.

mrjones2014 commented 2 years ago

I think I'll be able to come up with a solution using :h H

mrjones2014 commented 2 years ago

@mehalter can you test out on this branch: matjones/11-complex-layout-edge-cases

mehalter commented 2 years ago

Ok that fixed all the use cases I had before. Now I have a new one:

vertical split, the on the right window do 2 horizontal splits. The middle window up/down is backwards:

2022-03-23_14:31:47_screenshot

I can also confirm that this is new behavior on the branch and not on main

mrjones2014 commented 2 years ago

@mehalter okay try again?

mehalter commented 2 years ago

It's definitely getting better, it's really close I think! I am getting the error only with multiple horizontal splits.

Case: make at least 2 horizontal splits, the middle (non edge) windows are backwards resizing left/right. This only happens in a non edge horizontal split is on the top or bottom edge.

Here are example splits that show it:

Most basic 1x3 (highlighted not working):

2022-03-23_15:50:33_screenshot

3x3 (highlighted is working and the windows to the immediate top and bottom are not working:

2022-03-23_15:52:10_screenshot

mehalter commented 2 years ago

No matter how many horizontal splits I do the non edge touching windows resize left/right backwards unless it is fully surrounded by windows.

mrjones2014 commented 2 years ago

@mehalter 3rd time's the charm? I was checking top/bottom edge non-exclusively for the edge case, when I should have been checking "top or bottom edge but not both" for vertical splits, if that makes sense

mehalter commented 2 years ago

Hm, this fixes the 1x3 case I mentions above, but not the 3x3 or 2x3 case like this (either of the middle windows have left/right backwards:

2022-03-23_16:10:51_screenshot

Also thanks so much for debugging this :) I really appreciate all the hard work!

mrjones2014 commented 2 years ago

Note to self for the morning: try comparing the window positions with nvim_win_get_position and if the edge is passed the original window, move over to the original window

bradyslot commented 2 years ago

I can confirm I'm experiencing the same behavior as @mehalter, besides, good work on the plugin, it's nice to see someone wants the exact intended usage as myself. An additional issue I'm facing with this plugin is lag, it's responsive enough if I only need to resize a few characters, but holding down the key combo causes it to lags behind, this may be a neovim rendering limitation though. Also, I'm gonna be annoying and suggest this plugin listen to key repeats after the leader key is pressed (perhaps this should be a feature request). In the way I use tmux, I press the prefix key then use the arrow keys to resize the panes, but tmux will listen for like 500ms for key repeats, this allows me to make arbitrary sizes really easily. If it's not a feature you value, I may look into it myself and submit a pull request ;)

mrjones2014 commented 2 years ago

suggest this plugin listen to key repeats after the leader key is pressed

Sounds cool, would you mind filing a separate issue for that?

An additional issue I'm facing with this plugin is lag

I, uhh, basically ended up completely refactoring the plugin and I think currently itโ€™s doing way too much work to figure stuff out. Can you try the branch I mentioned above? It is not fully working correctly but just to see if it has the same lag.

bradyslot commented 2 years ago

The 'matjones/11-complex-layout-edge-cases' branch fixes the more complex layout issues, the lag is slightly improved, but not perfect. I will open another issue for that feature :)

mrjones2014 commented 2 years ago

@mehalter okay I'm pretty sure it should be fixed for all the cases listed now. @bradyslot can you tell me if there's still significant lag? I introduced an edge detection caching mechanism.

bradyslot commented 2 years ago

I'd say there's about a 30% subjective improvement in responsiveness, however, the movement is more jumpy when holding the shortcut down, as in I'm noticing it skipping steps. The skipping is more noticeable on the up and down than the left and right.

mrjones2014 commented 2 years ago

Hm. What kind of hardware are you on? I'm not noticing any lag whatsoever. I'm wondering if it's a performance issue or something else is going on.

bradyslot commented 2 years ago

I wouldn't be surprised, I seem to have had performance issues with alacritty. I'm on Arch with a R9 270x and all the relevant drivers installed. I'll investigate, thanks for your time :)

mehalter commented 2 years ago

@mrjones2014 I think all of the resizing is working great! :D There is one last thing that I can find that I had haven't experienced yet....

This is the layout: (3 vertical splits and in the middle section I did a horizontal split with 2 vertical splits in the middle lower section)

2022-03-24_08:44:44_screenshot

The highlighted section when I resize right or left it actually just moves me to the split above it.... no resizing at all.....

mehalter commented 2 years ago

oh I should mention that the up/down resizing on the highlighted region works as expected and all of the other splits in this layout resize correctly

mrjones2014 commented 2 years ago

Yup. Got it. I had a utility function to handle what's happening there, and forgot to use it in one case. It should be fixed now.

This has been one wild ride ๐Ÿ˜…

I appreciate all the back and forth though, because to fix this issue I ended up basically rewriting a lot of the plugin, and the code is way, way better and simpler and easier to read/follow now. The code is in a much much better state in #12

mehalter commented 2 years ago

No problem! I'm glad I could help with the debugging , I'm really looking forward to this plugin :) Also the latest commit on the branch hasn't resolved the issue for me

mrjones2014 commented 2 years ago

Hmmm, are you sure you're up to date with the latest commit on that branch? It fixed it for me, and I reproduced the layout from your screenshot ๐Ÿค”

If you're using Packer maybe you need to :PackerCompile?

mehalter commented 2 years ago

I am sure, I have also just started from a completely fresh system with ~/.local/share/nvim deleted as well as my compiled packer and reinstalled everything and I am still getting this behavior ๐Ÿค”

mrjones2014 commented 2 years ago

Interesting. I'm going to merge the PR since it's definitely a net improvement, but then I'll re-open this issue since you're still having that problem, and we can continue to investigate.

mehalter commented 2 years ago

Yeah I just pulled the new master and I am getting the same behavior with the previously described layout with resizing left/right moving me to the top split. Luckily this isn't a layout I ever find myself in so I think for my daily use it should be alright :joy;

mrjones2014 commented 2 years ago

I'm perplexed by the fact that I can't reproduce it though ๐Ÿค”

mehalter commented 2 years ago

Here is another layout I am getting it in that's simpler. Looks to be whenever I have a split to the right

This is a vertical split and then on the left side I did a horizontal split and then a vertical split on the lower section: 2022-03-24_09:35:57_screenshot

mehalter commented 2 years ago

I suppose it does still move the edge that I want to move :joy: it just moves me to a different split which is very unexpected behavior when resizing

bradyslot commented 2 years ago

Can confirm I'm experiencing the same jump to top window issue @mehalter is. On 23fd0f89b01efd3c77017e8ff30c25cda7122ef8

mrjones2014 commented 2 years ago

Can't reproduce it with that layout either ๐Ÿ˜ฉ

It seems like you might be having a cached version or something?? weird. Regardless, I can add the following to guarantee I don't move your cursor:

local cur_win_id = vim.api.nvim_get_current_win()
-- do all the resizing stuff
vim.api.nvim_set_current_win(cur_win_id)
mehalter commented 2 years ago

let me try refreshing my cache actually and see if that changes things

mehalter commented 2 years ago

hm I'm getting the same behavior even in a fresh VM

mrjones2014 commented 2 years ago

Can't reproduce it with that layout either ๐Ÿ˜ฉ

It seems like you might be having a cached version or something?? weird. Regardless, I can add the following to guarantee I don't move your cursor:

local cur_win_id = vim.api.nvim_get_current_win()
-- do all the resizing stuff
vim.api.nvim_set_current_win(cur_win_id)

hmmm. I just added this code. Is your cursor still jumping?

mehalter commented 2 years ago

oh now I am getting an error: 2022-03-24_09:40:58_screenshot

mrjones2014 commented 2 years ago

whoops, passed a param by mistake. try again.

mehalter commented 2 years ago

that has fixed it for me :)

bradyslot commented 2 years ago

that has fixed it for me :)

yay! me too!

mrjones2014 commented 2 years ago

Awesome! ๐ŸŽ‰ ๐ŸŒฎ

mehalter commented 2 years ago

@mrjones2014 I hate to be the bearer of bad news but I have found a new one.....

mehalter commented 2 years ago

Layout: horizontal split, then in the top half doing 2 vertical splits and a horizontal split in the upper middle window. The highlighted window up/down is backwards 2022-03-25_14:50:25_screenshot