feat: use relative = 'editor', add overlap options

[willothy]

When using plugins such as mini.animate, status windows on the right-hand side of the editor flicker and move when they should stay in place. This is because of the relative="win" option in the windows' config, which causes them to move automatically during resize at the same time as they're moved by the incline manager. Moving to relative="editor" and manually applying window position offsets fixes this, and results in much smoother movement.

This also allows the plugin to ignore the winbar for the top-level window if configured to do so.

I also added an option to overlap the tabline, in case anyone wants that (happy to remove that if it's not wanted).

Edit: closes #27

[willothy]

I do need to add to the help file, but the doc tests are failing at the checkout step and the errors from the schema validation tests are a bit incomprehensible. Mind giving some info on what changes would be required in the config/* files to make this work? Thanks :)

Edit: actually looks like similar errors are occurring on main, so not quite sure how to proceed here.

[b0o]

Thanks for these PRs. I'm going to look into the failing tests on main and will update you.

[willothy]

Sounds good!

[b0o]

Hi again! I'm sorry for the long delay, I was on vacation for about a month.

I fixed the failing tests and docs build on main. If you rebase on top of main, and then in test/config.lua, change your usages of Which.Is.A.DictLike to Which.Is.A.Table, the tests should pass.

---

Regarding the functionality, this change has a lot of edge cases to consider. There are many possible combinations of the following:

• Is the tabline visible?
• Is the winbar enabled/disabled?
• Is the statusline per-window, global, or disabled?
• Is the user's window.placement.vertical "top" or "bottom"?
• What is the user's window.margin.vertical?
  • If it's 0, we already have special overlap behavior (described here)
  • If it's > 0, how does this interact with window.overlap?
• If config.hide.cursorline is enabled, are we properly determining when to hide incline?

Here are a few issues I found:

1. Using this this config:

   require'incline'.setup {
     window = {
       placement = { horizontal = 'right', vertical = 'top' },
       overlap = {
         winbar = false,
         tabline = false,
       },
       margin = {
         horizontal = 0,
         vertical = 0,
       },
     },
     hide = {
       cursorline = 'focused_win',
     },
   }

   If 'winbar' is disabled, the position of incline in the top row of windows should be line 1, but it's on line 2:

2. Using this this config:

   require'incline'.setup {
     window = {
       placement = { horizontal = 'right', vertical = 'top' },
       overlap = {
         winbar = true,
         tabline = false,
       },
       margin = {
         horizontal = 0,
         vertical = 0,
       },
     },
     hide = {
       cursorline = 'focused_win',
     },
   }

   If 'winbar' is disabled, the position of incline is correct, but it's not hidden correctly according to hide.cursorline: (In the screenshot, the active cursor is on line 1 of baz.txt, so incline should be hidden in that window)

3. Using this config:

   require('incline').setup {
     window = {
       placement = { horizontal = 'right', vertical = 'top' },
       overlap = {
         winbar = false,
         tabline = true,
       },
       margin = {
         horizontal = 0,
         vertical = 1,
       },
     },
     hide = {
       cursorline = 'focused_win',
     },
   }

   When the user has the winbar enabled, the incline window is overlapping all of the winbars:

---

There are also a few ambiguities:

1. Using this config:

   require('incline').setup {
     window = {
       placement = { horizontal = 'right', vertical = 'top' },
       overlap = {
         winbar = true,
         tabline = true,
       },
       margin = {
         horizontal = 0,
         vertical = 0,
       },
     },
     hide = {
       cursorline = 'focused_win',
     },
   }

   When the user's tabline and winbar are both enabled, this is the result:

   

   Should the incline windows for bar.txt and qux.txt overlap the window border? Maybe we should add a window.overlap.borders option to control this (this would necessitate changing the old behavior described here).

2. Using this config:

   require('incline').setup {
     window = {
       placement = { horizontal = 'right', vertical = 'bottom' },
       margin = {
         horizontal = 0,
         vertical = 0,
       },
     },
     hide = {
       cursorline = 'focused_win',
     },
   }

   The placement is different than it was before your changes. Before it looked like this: After it looks like this:

   Is this the desirable behavior? This is actually similar to what #27 is requesting. Maybe we should have window.overlap.statusline and window.overlap.borders options as well to control this.

---

I know that's a lot to consider. Let me know if you have any thoughts or ideas. I really do like the direction this PR is going, I think if we can figure out these issues that it will be a big improvement over the current positioning behavior.

[willothy]

If you rebase on top of main, and then in test/config.lua, change your usages of Which.Is.A.DictLike to Which.Is.A.Table, the tests should pass.

Thanks! Will do.

Regarding the functionality, this change has a lot of edge cases to consider.

Definitely expected this to be the case, I'll start working through those this week. Thanks for the very detailed testing and info!

Maybe we should have window.overlap.statusline and window.overlap.borders options as well to control this.

I didn't intend for that to be the behavior of placement.vertical=bottom, but I think adding an option for it is a good idea. Should be trivial to implement as well.

If window.margin.vertical is 0, we already have special overlap behavior (described here)

Since this PR is already adding options for overlap, maybe this shouldn't be a special case anymore? Definitely seems more user-friendly to have a named option.

[b0o]

Since this PR is already adding options for overlap, maybe this shouldn't be a special case anymore? Definitely seems more user-friendly to have a named option.

I agree, it would be better for it to be an explicit option.

[willothy]

I'm a bit busy right now but I'll try to get to this in the next week or two :)

[willothy]

Oops I totally forgot about this PR, my bad! I definitely still want to move this forwards since I've been using it - I'll get started on those edge cases and the additional option ASAP.

[b0o]

Awesome, looking forward to it!

[willothy]

Oops I forgot about issue references lol my bad

[willothy]

We are gonna need a lot of tests to make sure this actually works, there are so many possible configs

[willothy]

Also the docs check is broken, failure is not related to this PR.

[willothy]

Alright, I think this should be ready for another review pass. There may be some more edge cases to figure out since there are a lot of possible configs.

Also I will definitely simplify the main changed area as much as possible, it's definitely messier than it needs to be atm - just wanted to get it working for now lol.

[b0o]

Thanks @willothy! I'll review this over the weekend. (Btw I disabled the docs workflow on PRs so that workflow failure should no longer appear)

[b0o]

Overall this is looking good and feels more responsive!

Some edge cases I noticed where cursorline hiding is off by one line:

1. When overlap.winbar = true and winbar is visible:

hide = {
  cursorline = true,
},
window = {
  margin = { horizontal = 0, vertical = 1 },
  placement = { horizontal = 'right', vertical = 'top' },
  overlap = {
    tabline = false,
    winbar = true,
    borders = false,
    statusline = false,
  },
}

2. When overlap.tabline = true and tabline is not visible (showtabline is 0, or 1 with only one tabpage):

hide = {
  cursorline = true,
},
window = {
  margin = { horizontal = 0, vertical = 0 },
  placement = { horizontal = 'right', vertical = 'top' },
  overlap = {
    tabline = true,
    winbar = false,
    borders = false,
    statusline = false,
  },
  padding = 0,
  zindex = 49,
  winhighlight = {
    active = { Normal = 'Normal' },
    inactive = { Normal = 'Normal' },
  },
}

It would definitely be nice to figure out how to unit test the placement and hiding since there are so many edge cases, but we can figure that out later.

Other than that I think it looks good!

[willothy]

Cool, glad it's working better than before! I'll look into those cursorline issues today.

[willothy]

Ok I think I've fixed that edge case with the tabline, and fixed another I found where if both tabline and winbar were hidden the window would not be visible.

[b0o]

Nice! Seems like there's only one edge case now: When overlap.winbar = false and winbar is visible, cursorline hiding is off by one.

[willothy]

Awesome! I'll try and get that fixed today. I think I might've broken that with the last fix lol

[willothy]

Ahhhh I hope that didn't re-break something else lol

[b0o]

That still isn't quite working. This seems to fix it:

diff --git a/lua/incline/winline.lua b/lua/incline/winline.lua
index c687b8d..aa2ceed 100644
--- a/lua/incline/winline.lua
+++ b/lua/incline/winline.lua
@@ -70,7 +70,7 @@ function Winline:get_win_geom_row()
     -- statusline if laststatus is not 3

     -- TODO(willothy): this can obviously be simplified a lot, there is a good bit of repetition
-    if vim.o.laststatus ~= 3 or a.nvim_win_get_position(self.target_win)[1] <= 1 then
+    if a.nvim_win_get_position(self.target_win)[1] <= 1 then
       if cw.margin.vertical.top == 0 then
         if
           config.window.overlap.tabline
@@ -223,7 +223,8 @@ function Winline:render(opts)
   end
   if
     (config.hide.cursorline == true or (config.hide.cursorline == 'focused_win' and self.focused))
-    and self:get_win_geom_row() == a.nvim_win_call(self.target_win, vim.fn.winline)
+    and (self:get_win_geom_row() + ((vim.wo[self.target_win].winbar == '') and 1 or 0))
+      == a.nvim_win_call(self.target_win, vim.fn.winline)
   then
     self:hide(HIDE_TEMP)
     return

Would you mind testing that and, if it seems to work for you, making a commit?

[willothy]

Looks like that works!

[b0o]

Awesome, thank all of your work and persistence!

[willothy]

Super exciting! Took a while but I think the flexibility is awesome.