fdschmidt93 / telescope-egrepify.nvim

Variable user customization for telescope.live_grep to set rg flags on-the-fly
MIT License
122 stars 13 forks source link

feat(entry_maker): highlight results with treesitter #23

Closed fdschmidt93 closed 4 months ago

fdschmidt93 commented 1 year ago

Enable treesitter highlighting of results buffer. In practice, I don't notice any slowdown

Known issue: scratch buffer when nvim is opened does not get closed

@delphinus @otavioschwanck I would greatly appreciate if you hop on this branch and give it a whirl for feedback! Thanks in advance if you do so :) Though I'll plan on making the feature opt-in anyways.

Needs results_ts_hl = true in config.

image

Update:

The big problem here is getting performance right. There are are quite a few scenarios in which tree-sitter highlighting would lead to major blockage. The best would be some sort of timeout for highlighting a particular file but tree-sitter parsing cannot be cancelled once started. The best solution that comes to mind would be to spawn an embedded nvim instance when the picker opens, ask that one to parse the file and send the parsed tree to the main instance, since, if we don't receive the tree in time, we can just proceed and ignore the tree even once we received it (which doesn't block neovim as much).

otavioschwanck commented 1 year ago

am i dreaming ? Awesome feature!

otavioschwanck commented 1 year ago

@fdschmidt93 i throws an error when i tried to grep:

[telescope] [WARN  18:11:47] /Users/otavio/.local/share/nvim/lazy/telescope.nvim/lua/telescope/pickers.lua:642: Finder failed with msg:  ....nvim/lua/telescope/_extensions/egrepify/entry_maker.lua:100: attempt to call field 'iter' (a nil value)
fdschmidt93 commented 1 year ago

Ah, that's a nightly API. Will change that. Thanks!

fdschmidt93 commented 1 year ago

Ideally fixed now!

otavioschwanck commented 1 year ago

Ideally fixed now!

The performance is awesome! working great now!

A possible improvement is the "match" highlight. When the highlight happens, it loses the colors. The ideal is just change the background or something like that:

image

EDIT about the performance:

For my big project, it slow considerable for some reason. could have an option to just add the highlight for TS for the first X matches, what you think @fdschmidt93 ?

otavioschwanck commented 1 year ago

When typing to search a term in a big project, it slows considerable (when return 1000 results or more), its pretty common to happen. i think the idea here is to add the highlight for the first 50 matches or something like that

fdschmidt93 commented 1 year ago

A possible improvement is the "match" highlight. When the highlight happens, it loses the colors. The ideal is just change the background or something like that:

I'm not sure I 100% follow. If I understand you correctly:

So long story short, very likely not possible right now.

For my big project, it slow considerable for some reason. could have an option to just add the highlight for TS for the first X matches, what you think fdschmidt93 ?

Right now it only tries to highlight entries that appear in your results list and caches highlights for those files. One approach would be to increase some debounce (in ms) to the picker so the results entries don't get rerendered too often so the load is a bit spread out.

Common slow downs I'd expect:

Highlight parsing is probably the one that could be most improved. Might do it in a follow-up PR.

When typing to search a term in a big project, it slows considerable (when return 1000 results or more), its pretty common to happen. i think the idea here is to add the highlight for the first 50 matches or something like that

That's essentially already happening.

I can reproduce slowdown now. Will try to improve performance such that it becomes acceptable.

otavioschwanck commented 1 year ago

i found another bug @fdschmidt93 , sometimes, after you select the file, it open the file without highlight

delphinus commented 1 year ago

Nice! It seems to work fine for me. I can check results more clearly. I will test for a while.

delphinus commented 4 months ago

I've been using recent months, and really been satisfied with this. Can you merge this? @fdschmidt93

fdschmidt93 commented 4 months ago

I'll merge this since it's opt-in anyways but I won't have any time to work/improve this myself until some time in August, to be honest :sweat_smile:

But I trust your stress-testing @delphinus