add multi window support #419

xzbdmw · 2024-03-19T07:12:59Z

This will track a global map that map winid to {bufnr,gut_bufnr, context_bufnr,gutter_winid,context_winid} and dynamically clean up and add new one, I'm new to lua so thanks for your advice. Currently WinScroll autocmd seems don't adjust context window position and I can't figure out why.

xzbdmw · 2024-03-19T07:15:06Z

Oh why the formatting is changed

lua/treesitter-context/render.lua

romgrk

I haven't tested yet but overall the approach & feature looks good to me.

romgrk · 2024-03-19T10:43:13Z

lua/treesitter-context.lua

+  autocmd({ 'WinScrolled' }, function()
+    for winid, bufnr_map in pairs(require('treesitter-context.render').get_bufnr_maps()) do
+      update(bufnr_map.bufnr, winid)
+    end
+  end)


Are we updating all the windows on scroll? Shouldn't we be only updating the scrolled one?

Commands such as split will trigger winscroll, and every window width should be adjusted to new width, how should we differentiate scroll and resizing? Should I use winresized instead? Ideally if user create a new split, I’d like to close all of them and reopen again to avoid position flashing, will try again.

lua/treesitter-context.lua

lua/treesitter-context/render.lua

xzbdmw · 2024-03-20T04:38:23Z

There is one thing I can't understand
in neovide, resizing works perfect:

iShot_2024-03-20_12.35.51.mp4

but in kitty(as well as wezterm), when new window created, the position becomes wrong(if I focus the window again, position will be correct):

iShot_2024-03-20_12.37.36.mp4

Can you help with me what causes the difference?
note: I checked the else branch in display_window, It indeed changed context's window width after resize, but the real effect don't show up.

xzbdmw · 2024-03-21T08:21:26Z

@romgrk @lewis6991 Hello could you test this? The only issue remains is the attached[buf] I'm not sure how to change it, since now we have multiple windows.

Edit: I found

local update = throttle(function()
  local bufnr = api.nvim_get_current_buf()
  local winid = api.nvim_get_current_win()

these two lines have race conditions that nvim_get_current_buf sometimes returns the old bufnr which is not the one in nvim_get_current_win's buf, add a 1ms delay calling update fix that(vim.schedule also works). Not sure why it is inconsistent.

  autocmd({ 'WinScrolled', 'BufEnter', 'WinEnter', 'VimResized' }, function()
    vim.defer_fn(function()
      update()
    end, 1)
  end)

romgrk · 2024-03-22T13:32:43Z

For your issue, I don't like adding an async delay there, even if just 1ms. You should retrieve the buf/win ID during the event handler via the usual mechanism (<amatch> and friends) and pass it as an argument to update(), maybe an optional param would be good. Though I don't see the defer_fn in this PR, lmk when it's ready for review.

lewis6991 · 2024-03-26T19:31:58Z

lua/treesitter-context.lua

+    do
+      for _, window_id in pairs(window_ids) do
+        if stored_winid == window_id then
+          throttle(function()


This doesn't work for anonymous functions since it is only ever called once.

Hmm, maybe no need for throttle? Just directly calling
open() to update state, and the same for WinEnter, since some plugin may open many windows in a short time , we should instantly trigger open().

Calling throttle like this (throttle(...)()) doesn't work. It needs to be.

local f = throttle(...) f() f() -- throttled

Not:

throttle(...)() throttle(...)() -- not throttled!

lewis6991 · 2024-03-26T19:32:50Z

lua/treesitter-context/render.lua

-  return buf
+--- @param winid integer?
+local function win_close(winid)
+  vim.schedule(function()


The code doesn't show why this is needed.

I move win_close to top since store_context needs to call close if underline buffer changed

xzbdmw · 2024-03-26T21:51:22Z

lua/treesitter-context.lua

@@ -155,7 +198,18 @@ function M.enable()

  autocmd({ 'BufLeave', 'WinLeave' }, close)


I think maybe bufleave and winleave is no longer needed, winleave can be replaced by winclosed, and bufleave's close behavior is expressed in store_contest's win_close call.

xzbdmw · 2024-03-26T22:29:13Z

lua/treesitter-context/render.lua

      relative = 'win',
      width = width,
      height = height,
      row = 0,
      col = col,
    })
  end
-  return winid
+  return float_winid
 end

 --- @param winid integer


textoff is inconsistent too. In my case it will return 4 and after next update time it will return 8, cause window position to flash(mark the wrong line number, two lines below)

xzbdmw · 2024-04-08T17:26:30Z

@romgrk hello do you have time to review?
According to this suggestion
https://www.reddit.com/r/neovim/comments/10eu7go/comment/j4uaxpe/?utm_source=share&utm_medium=web3x&utm_name=web3xcss&utm_term=1&utm_content=share_button
I'm wrapping WinEnter in a vim.schedule now to get new bufnr. It should be ready to merge, after 3 weeks of use I see no problem.
But I have not find a reliable way to get buffer information after WinEnter is triggered, the solution above is not perfect if you open many windows at the same time, since vim.schedule's execute time is uncertain, as the origin comment says. But it is still a improvement since there would not be some file buffer unrelated windows to show previous buf's context.

romgrk · 2024-04-08T18:19:45Z

lua/treesitter-context/render.lua

+    if window_ctx.bufnr == bufnr then
+      return window_ctx
+    else
+      -- Unserline buffer have changed, close it


romgrk

LGTM, I'll run the PR in my config to test it.

romgrk · 2024-04-08T18:41:51Z

lua/treesitter-context.lua

I'm wrapping WinEnter in a vim.schedule now to get new bufnr. It should be ready to merge, after 3 weeks of use I see no problem.
But I have not find a reliable way to get buffer information after WinEnter is triggered, the solution above is not perfect if you open many windows at the same time, since vim.schedule's execute time is uncertain, as the origin comment says. But it is still a improvement since there would not be some file buffer unrelated windows to show previous buf's context.

Yeah sometimes events are ugly. Maybe this would be better fixed in neovim directly, if it's unreliable.

Hi, I remove WinEnter since it is not reliable, and indeed there are no need for that autocmd

Wdym by not needed? I'm observing a few cases where the context doesn't show up, wondering if it's connected?

I think BufEnter is more general than WinEnter, since there must be a loaded buffer to show context, which case context don't show up? Could you try to add WinEnter to see if it still doesn't show up?

Have you observed similar behavior? Can you do the changes on this PR? Could you also explain why it was needed to change the events we listen to?

Also I've been using for 2 weeks and I love how much less flickering there is from the context window opening/closing on every window change.

Have you observed similar behavior

No, but I have context window opened in none-file buffers if I just keep WinEnter unchanged as is.

I have a keymap that first enter right "reference list", then immediately move focus to the preview window, and a workaround is wrapping callback in vim.schedule to get the right buffer number(but it is not always right theoretically that's why I delete it), If you prefer keep WinEnter I'll add it back.

I don't have a preference, whatever works is best, I'm just wondering if changing the events might have caused the few cases I saw where the window wasn't appearing. I'm always very confused with autocmds so I couldn't tell you which combination of BufEnter/WinEnter/BufWinEnter is right for our use-case.

Well, have added it back.

WinEnter may mess up windows and there is no need for that I think

lua/treesitter-context/render.lua

lua/treesitter-context.lua

xzbdmw · 2024-05-03T21:49:06Z

@romgrk @lewis6991 Hello any chance this get merged?

romgrk · 2024-05-07T19:52:59Z

Code-wise the PR looks good, but I've been running it for a while and I keep seeing cases where the context window(s) don't get opened. It doesn't bother me much personally but I don't have enough confidence in it to give it a green light, and I also haven't had time to debug the code.

Have you seen any such cases?

xzbdmw · 2024-05-07T20:21:46Z

Hmm no problem for me so far, works seamless( I mainly use nightly), if users encounter the same problem, they may provide some information and we can fix it. I think whenever you move cursor, context will open if it didn't open before.

apollo1321 · 2024-08-21T17:09:57Z

@romgrk @lewis6991 Hi, could you please clarify the reason this PR was closed?

I have developed my own implementation of multi-window support a couple of months ago, and I have been using it extensively since then. My experience has been very positive; it has performed flawlessly without any issues related to multiple windows or resizing.

Additionally, I have conducted performance testing using flamegraphs, and I have observed that treesitter-context consumes approximately 7% of processing time, which appears to be within a reasonable range.

I believe that multi-window support is a highly valuable feature, especially for working with large codebases efficiently. I think it would greatly benefit other users as well.

If it would be helpful, I am more than happy to share my implementation and potentially create a new PR.

xzbdmw · 2024-08-21T17:12:34Z

Because I tried to rebase but there are too many commits behind 😄 I planned to open a new pr but forgot it, it will be awesome if you are happy to create a pr.

romgrk · 2024-08-21T23:42:21Z

Not enough time to review/debug the few minor issues that appeared at the time. I'd be happy to see this feature, I just don't have much time to put into FOSS right now, and @lewis6991 also needs to sign off on it as he's the more active maintainer at the moment. But please open a PR if you have one that's production ready.

add multi window support

cbbd191

fix typo

132a3cf

romgrk reviewed Mar 19, 2024

View reviewed changes

lua/treesitter-context/render.lua Outdated Show resolved Hide resolved

romgrk reviewed Mar 19, 2024

View reviewed changes

lua/treesitter-context/render.lua Outdated Show resolved Hide resolved

romgrk reviewed Mar 19, 2024

View reviewed changes

lua/treesitter-context/render.lua Outdated Show resolved Hide resolved

romgrk reviewed Mar 19, 2024

View reviewed changes

lewis6991 reviewed Mar 19, 2024

View reviewed changes

xzb added 3 commits March 20, 2024 12:09

make winresized change all context's position

669d63a

make winresized change all context's position

cfa7489

fix formatting change

ea892ff

xzb added 8 commits March 20, 2024 12:51

forget change nvim_get_current_buf in context_range, fix it

e987c2d

fix lua doc

946342f

delete winleave autocmd

201bd84

comment out bufleave and winleave

8b06f87

bufleave and winleave is needed

6b1db19

close context window if underline buffer changed

2707a4b

close context window if underline buffer changed

9f7ef22

fix indent

0fb5676

xzbdmw force-pushed the master branch from 5e0c428 to 0fb5676 Compare March 20, 2024 06:51

xzb added 3 commits March 20, 2024 17:25

move function close to top

088b6e1

fix type anotation

d9e11d4

fix: add relative win at display window

c388de5

xzbdmw mentioned this pull request Mar 21, 2024

feature request: Add an api for keymap triggered callback cbochs/portal.nvim#47

Closed

xzbdmw added 2 commits March 22, 2024 15:31

fix: add validation in bufnr and winid

5c05be6

fix: adjuest indent

0346e53

lewis6991 reviewed Mar 26, 2024

View reviewed changes

xzbdmw commented Mar 26, 2024

View reviewed changes

delete bufleave and winleave and change throttle usage

3842823

xzbdmw commented Mar 26, 2024

View reviewed changes

use vim.schedule in WinEnter to get new bufnr

12a266f

romgrk reviewed Apr 8, 2024

View reviewed changes

romgrk approved these changes Apr 8, 2024

View reviewed changes

romgrk reviewed Apr 8, 2024

View reviewed changes

fix typo

3a02b7b

xzbdmw mentioned this pull request Apr 10, 2024

treesitter-context integration otavioschwanck/arrow.nvim#43

Merged

Add BufWinEnter to make :e work, remove WinEnter

7a2aa0e

WinEnter may mess up windows and there is no need for that I think

romgrk reviewed Apr 11, 2024

View reviewed changes

lua/treesitter-context/render.lua Outdated Show resolved Hide resolved

rename window_contexts

9e7547a

romgrk reviewed Apr 19, 2024

View reviewed changes

lua/treesitter-context.lua Outdated Show resolved Hide resolved

romgrk reviewed Apr 19, 2024

View reviewed changes

lua/treesitter-context.lua Outdated Show resolved Hide resolved

xzbdmw added 2 commits April 19, 2024 21:40

extract logic to function and add winenter

4d5e75f

recreate buffer if user turn off line numbers and show it again

e361d80

xzbdmw added 3 commits May 18, 2024 14:19

use correct winid for computing relative linenum

485f48b

Update render.lua

0e8438f

Merge branch 'master' into multi_window

ffd44d3

xzbdmw closed this Jul 9, 2024

apollo1321 mentioned this pull request Aug 25, 2024

feat: multiwindow support #480

Closed

		@@ -155,7 +198,18 @@ function M.enable()

		autocmd({ 'BufLeave', 'WinLeave' }, close)

Uh oh!

add multi window support #419

add multi window support #419

Uh oh!

Conversation

xzbdmw commented Mar 19, 2024

Uh oh!

xzbdmw commented Mar 19, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

romgrk left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xzbdmw Mar 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xzbdmw commented Mar 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xzbdmw commented Mar 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

romgrk commented Mar 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xzbdmw Mar 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lewis6991 Mar 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xzbdmw Mar 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xzbdmw commented Apr 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

romgrk left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

romgrk Apr 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xzbdmw Apr 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

xzbdmw Mar 19, 2024 •

edited

Loading

xzbdmw commented Mar 20, 2024 •

edited

Loading

xzbdmw commented Mar 21, 2024 •

edited

Loading

romgrk commented Mar 22, 2024 •

edited

Loading

xzbdmw Mar 26, 2024 •

edited

Loading

lewis6991 Mar 26, 2024 •

edited

Loading

xzbdmw Mar 26, 2024 •

edited

Loading

xzbdmw commented Apr 8, 2024 •

edited

Loading

romgrk Apr 19, 2024 •

edited

Loading

xzbdmw Apr 19, 2024 •

edited

Loading

xzbdmw commented May 7, 2024 •

edited

Loading

apollo1321 commented Aug 21, 2024 •

edited

Loading

xzbdmw commented Aug 21, 2024 •

edited

Loading