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

Feature: nvim-treesitter-context in previewer [DONE] #1552

Closed
1 task done
ibhagwan opened this issue Dec 6, 2024 · 18 comments
Closed
1 task done

Feature: nvim-treesitter-context in previewer [DONE] #1552

ibhagwan opened this issue Dec 6, 2024 · 18 comments

Comments

@ibhagwan
Copy link
Owner

ibhagwan commented Dec 6, 2024

Have you RTFM'd?

  • I have done proper research

Feature Request

Ref: #1485, #1509

Enabled by default with the below options

require("fzf-lua").setup({
  previewers = { builtin = {
    -- See https://github.com/nvim-treesitter/nvim-treesitter-context for more options
    -- Set `context=true` to use whatever options used in your `nvim-treesitter-config` setup call
    treesitter = { context = { max_lines = 1, trim_scope = "inner" } }
  } }
})

Can also be set per call:

Note that flag "path" for a call is previewer.treesitter.context

:FzfLua live_grep previewer.treesitter.context=true
:FzfLua live_grep previewer.treesitter.context={max_lines=2,trim_scope="outer"}

Or with lua:

:lua require("fzf-lua").live_grep({ previewer = { treesitter = { context = true } } })
:lua require("fzf-lua").live_grep({ previewer = { treesitter = { context = {max_lines=2,trim_scope="outer"} } } })
@ibhagwan ibhagwan added the feature request New feature label Dec 6, 2024
ibhagwan added a commit that referenced this issue Dec 8, 2024
To enable
```lua
require("fzf-lua").setup({
  previewer = { builtin = { treesitter = { context = true} } }
})
```
ibhagwan added a commit that referenced this issue Dec 8, 2024
To enable
```lua
require("fzf-lua").setup({
  previewer = { builtin = { treesitter = { context = true} } }
})
```
ibhagwan added a commit that referenced this issue Dec 8, 2024
To enable
```lua
require("fzf-lua").setup({
  previewer = { builtin = { treesitter = { context = true} } }
})
```
ibhagwan added a commit that referenced this issue Dec 8, 2024
To enable
```lua
require("fzf-lua").setup({
  previewer = { builtin = { treesitter = { context = true} } }
})
```
ibhagwan added a commit that referenced this issue Dec 8, 2024
To enable
```lua
require("fzf-lua").setup({
  previewers = { builtin = { treesitter = { context = true} } }
})
```
@ibhagwan
Copy link
Owner Author

ibhagwan commented Dec 8, 2024

5038142

@xzbdmw I think I nailed it, no need to configure multiwindow or zindex as the code will temporarily set the required values for the lifetime of the fzf-lua window, seems to work pretty nice.

I couldn't figure out one thing, when first calling update on the new buffer the context refuses to show unless I scroll or switch to another entry within the same buffer, this seems to be solved by wrapping the update call with vim.schedule which is somehwat of a hack as it's not needed as we're not inside a vim.fastevent, but as long as it works I'm fine with it as the "integration" is a hack to begin with due to the lack of official API.

Add this to your setup call to enable (modifying the params of the "builtin" previewer):

require("fzf-lua").setup({
  previewers = { builtin = { treesitter = { context = true} } }
})

Or individually:

:FzfLua live_grep previewer.treesitter.context=true

Let me know what you think?

@xzbdmw
Copy link

xzbdmw commented Dec 8, 2024

Yes, it works for me, the reason you need some defer probably is that it is throttled(not that sure, it seems calling open() directly bypass the throttle mechanism), but schedule may not be enough, since the default throttle time is 150ms , a more reliable/hacky way is calling update() multiple times with defer_fn({0ms,10ms,100ms,200ms}), the cost of update() is very low.

@ibhagwan
Copy link
Owner Author

ibhagwan commented Dec 8, 2024

Yes, it works for me, the reason you need some defer probably is that it is throttled, but schedule may not be enough, since the default throttle time is 150ms , a more reliable/hacky way is calling update() multiple times with defer_fn({0ms,10ms,100ms,200ms}), the cost of update() is very low.

I used defer_fn first but as I saw it was working with 0 delay I switched to schedule, I’ll improve the “hack” tomorrow and also add an optional key bind to toggle the context on/off while the preview is open.

@xzbdmw
Copy link

xzbdmw commented Dec 8, 2024

Ok, I know the problem, when you call open() without defer, it indeed succeeds, the window opens, but some autocmds triggers update_single_context, it then checks cannot_open, but because the direct call to open(), attach[bufnr] will return false, causing the newly opened window to be immediately closed.

Comment out OptionSet and WinResized turned out to solve the defer hack.

While attach is a local variable, it seems we have no workaround for that, using defer_fn({0ms, 10ms, 100ms, 200ms}) still seems to be the easiest solution. Or, find a way to trigger the BufReadPost, so previewer buffer is added to attach.

@ibhagwan
Copy link
Owner Author

ibhagwan commented Dec 8, 2024

Ok, I know the problem, when you call open() without defer, it indeed succeeds, the window opens, but some autocmds triggers update_single_context, it then checks cannot_open, but because the direct call to open(), attach[bufnr] will return false, causing the newly opened window to be immediately closed.

Comment out OptionSet and WinResized turned out to solve the defer hack.

While attach is a local variable, it seems we have no workaround for that, using defer_fn({0ms, 10ms, 100ms, 200ms}) still seems to be the easiest solution. Or, find a way to trigger the BufReadPost, so previewer buffer is added to attach.

I tried using vim.o.eventignore = "all" but it didn't work, so it looks like this is currently the best way?

I didn't look explore the treesitter-context code deeply, what are the chances that a single call to vim,schedule doesn't show the window? It seemed to work pretty consistently for me with a single vim.schedule, perhaps on a slow machine?

I also added 2 more treesitter-context life-cycle options which IMHO are much nicer when it comes to the preview window, this makes it so there's less clutter when the context contains inner conditions and other nonsense, I believe what most users would care about is the calling function:

max_lines = 1, -- How many lines the window should span. Values <= 0 mean no limit.
trim_scope = 'inner', -- Which context lines to discard if `max_lines` is exceeded. Choices: 'inner', 'outer'

fc220ec

@xzbdmw
Copy link

xzbdmw commented Dec 8, 2024

what are the chances that a single call to vim,schedule doesn't show the window

If OptionSet/WinSrolled is guaranteed to triggered first, then it is scheduled to close any unattached window, after that, fzf-lua calls update, also scheduled, avoid the potential closing, there is no problem; on the flip side, if fzflua calls update() first, it will be closed by a later OptionSet/WinSrolled trigger, that's why it must be deferred by a schedule now. The order is not clear to me though.

@ibhagwan
Copy link
Owner Author

ibhagwan commented Dec 8, 2024

what are the chances that a single call to vim,schedule doesn't show the window

If OptionSet/WinSrolled is guaranteed to triggered first, then it is scheduled to close any unattached window, after that, fzf-lua calls update, also scheduled, avoid the potential closing, there is no problem; on the flip side, if fzflua calls update() first, it will be closed by a later OptionSet/WinSrolled trigger, that's why it must be deferred by a schedule now. The order is not clear to me though.

Perhaps we can leave one defer call with a timeout of 0 (equivalent to a single vim.schedule) and also enable this option by default - this will be the only way to get true feedback :)

If the single call fails, we can restore to multiple timely delayed calls.

@xzbdmw
Copy link

xzbdmw commented Dec 8, 2024

Yeah, I think it can be enabled by default, maybe defer_fn(10)? The race possibility is going to decrease a lot.

@ibhagwan
Copy link
Owner Author

ibhagwan commented Dec 8, 2024

Yeah, I think it can be enabled by default, maybe defer_fn(10)? The race possibility is going to decrease a lot.

You’re probably right, although I’d still prefer to have a call with 0 delay so it appears to display instantaneously, I’ve compromised on 2 calls with delays of 0, 20ms respectively and enabled by default.

617383a

@xzbdmw
Copy link

xzbdmw commented Dec 8, 2024

this is a new function added a month ago, open, get, close exits for over one year, maybe we should be very conservative for their existence, actually I test my treesitter-context fork in March's commit, and it works too, except the call to close_leaked_window, we can simply delete this line, check if open, get, close exists, only so, enable it successful.

max_lines = 1, -- How many lines the window should span. Values <= 0 mean no limit.
trim_scope = 'inner', -- Which context lines to discard if max_lines is exceeded. Choices: 'inner', 'outer'

For languages like java or rust, method lives inside impl or class keyword, maybe we can expose these settings to users.

@ibhagwan
Copy link
Owner Author

ibhagwan commented Dec 8, 2024

this is a new function added a month ago, open, get, close exits for over one year, maybe we should be very conservative for their existence,

I thought about it too, but I believe someone updating fzf-lua will also update treesitter-context.

For languages like java or rust, method lives inside impl or class keyword, maybe we can expose these settings to users.

I will make the context var accept a table which will act as the treesitter context config.

I’m going to give this a while to collect feedback and make the changes, I’m pretty sure users will also want the toggle keybind.

ibhagwan added a commit that referenced this issue Dec 8, 2024
New default treesitter previewer opts:
```lua
require("fzf-lua").setup({
  previewers = { builtin = {
    treesitter = { context = { max_lines = 1, trim_scope = "inner" } }
  } }
})
```

Also updated README.md with optional plugin deps for markdown.
@ibhagwan
Copy link
Owner Author

ibhagwan commented Dec 8, 2024

@xzbdmw, 1a3c312

The defaults are:

  context = { max_lines = 1, trim_scope = "inner" }

Set context=true to use whatever defaults you have setup original for nvim-treesitter-context.

Can also setup temporarily in one call, e.g.:

:FzfLua live_grep previewer.treesitter.context={max_lines=2,trim_scope="outer"}

ibhagwan added a commit that referenced this issue Dec 8, 2024
Customizable via `keymap.builtin` as action `toggle-preview-ts-ctx`

Ref: #1552
@ibhagwan
Copy link
Owner Author

ibhagwan commented Dec 9, 2024

c40666b added a new action toggle-preview-ts-ctx by default bound to <F7> and customizable via keymap.builtin.

Unless I'm missing something this feature is pretty much complete.

@TheLeoP
Copy link
Contributor

TheLeoP commented Dec 9, 2024

c40666b removed the ["ctrl-q"] = "select-all+accept", keymap from the telescope profile. Was this intentional or an oversight when modifying all of the keymaps?

@ibhagwan
Copy link
Owner Author

ibhagwan commented Dec 9, 2024

c40666b removed the ["ctrl-q"] = "select-all+accept", keymap from the telescope profile. Was this intentional or an oversight when modifying all of the keymaps?

Unintentional, while searching for keymap in profiles (due to adding of F7) I changed the profile keymap to inherit from defaults (thus deleting all defaults), this was one a mistake.

@ibhagwan
Copy link
Owner Author

ibhagwan commented Dec 9, 2024

@TheLeoP 498459c

@TheLeoP
Copy link
Contributor

TheLeoP commented Dec 9, 2024

Thanks @ibhagwan :D

@ibhagwan ibhagwan changed the title Feature: exlpore support for displaying nvim-treesitter-context in previewer Feature: nvim-treesitter-context in previewer [DONE] Dec 9, 2024
@ibhagwan
Copy link
Owner Author

2bc454b

With the addition of f7|f8 to increase/decrease the treesitter context max_lines, this feature is complete.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants