telescope.nvim
telescope.nvim copied to clipboard
feat(pickers): implement group-by.
Description
This PR implements a feature to enable having a grouping of entries have a "header" indicating the common field. This is very similar to what telescope-egrepify does display-wise, but fully configurable to allow grouping by arbitrary entry properties (to be fully honest, I don't right now know when I'd use this with anything than filename
, but making this general was easy enough), and to allow the customization of the display of those headers.
Noteworthy points:
- This does not impact the order of entries. If it turns out that entries from the same group end up separated by other entries, you'll get two groupings with two separate headers for them. When I was starting this, I was planning to have this affect the order, but gave up on that idea very quickly; it's possible that because of that, the name "group by" is no longer deserved; if that's the case, then I'm open to other name suggestions.
- The majority of the small, scattered changes implement a workaround for neovim/neovim#16166. In general, the headers are just displayed with an extmark, with
virt_lines
set to the header text, andvirt_lines_above = true
. However, for the very first header that does not currently work due to the neovim bug, so as a workaround I insert an additional blank line into the results buffer, and place an overlay extmark there. This complicates some of the math, but after fighting many, many off-by-one errors, I think I've got it right at this point? - There's a drive-by fix for an issue that happens on the master branch of telescope itself, where in some cases setting the ascending sorting direction would make telescope leave garbage entries at the bottom of the buffer (they weren't selectable, but would show up, without any highlights - I guess this is Halloween-appropriate?). It was bothering me a lot, so one of the changes in
_clear_extra_rows
makes sure that the ascending sorting direction still has its extra rows cleared when it is atmax_results
.
Type of change
- Bug fix (non-breaking change which fixes an issue)
- New feature (non-breaking change which adds functionality)
Resolves #2297
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list relevant details about your configuration
Extensively manually tested a roughly complete cartesian product of:
-
live_grep
andlsp_references
- with the
group_by
argument being a string, or a table with overrides - with and without a default configuration
- with both sorting strategies
- with and without devicons (for both the entries and the headers)
- with and without
path_display = "hidden"
Configuration:
- Neovim version (nvim --version):
NVIM v0.10.0-dev-324fad1
- Operating system and version: Debian unstable
Checklist:
- [x] My code follows the style guidelines of this project (stylua)
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas (I don't think it needs much more in terms of comments, but I'm happy to add comments wherever you deem them to be necessary)
- [x] I have made corresponding changes to the documentation (lua annotations)
@griwes thanks so much for the implementation, i am quite unfamiliar with lua, do you know how can someone try this code in their personal neovim config?
Please @Conni2461 could you review this PR? This feature would be awesome, order the results by filename just like VsCode.
I'm a little per-occupied to review bigger features at the moment but rest assured I haven't forgotten about this.
@jugarpeupv probably just replacing nvim-telescope/telescope.nvim
with griwes/telescope.nvim
and setting branch to group-by
in your package manager.
If more people want to give this a test, that'll be appreciated as well.
@jamestrew i tried to test this with your indications but an error arises, i do not understand why (see the image)
@griwes thank you for the implementation, I was wondering if you could provide an example on how to invoke the live_grep command, i was reading the code and i guess i would figure it out but i would appreciate it since I am still not quite familiar with how telescope pickers work.
@jugarpeupv you didn't write the correct branch name you used _ instead of -
@max397574 thank you! i managed to get it to work, it is just what i was looking for
@jamestrew hope this get merged soon
@griwes one thing i would suggest is to hide fileicon on the registries, since we can see the filetype icon on the header. It is just to see less icons which does not supply relevant information
@jugarpeupv you can also pass in path_display = 'hidden'
to the picker. The reason I didn't do anything with that option is that it already exists and seems fairly orthogonal to the feature at hand.
As to extensions, if they accept and pass through the arguments passed into them to the picker, this should also work there. If they don't, there really isn't much that can be done other than modifying the extensions.
@griwes I just tested with path_display but i do not see the results :S
As to extensions, i deleted the comment because i managed to get it working with the extension live_grep_args, just passing group_by = "filename", so it is fine
Thank you for your quick reply ^^
@griwes i just added path_display = "hidden" to the telescope global config and i can see the result, but this is not what i was talking about. I was pointing that the icons shown in the registries might be omitted, but in the registries, not in the header
Ah right, I forgot that path_display doesn't hide the icons; there's a separate option called disable_devicons
that you can set to true to hide them. (The group_by object also accepts disable_devicons
separately from that, to control just the icons in the header.)
i don't think this should be in the documentation so it doesn't get to big but perhaps after the pr is merged could you provide an example on how one would write a custom group-by function (assuming that is possible) e.g. for grouping by filetype in find_files
@griwes disable_devicons is working as i expected, thank you.
@jamestrew any chance you could give this a cursory glance soon-ish? I haven't looked at what the current conflicts are yet, but I'd like to rebase this in the near future and would love some initial feedback, specifically as to whether the approach in this PR is generally acceptable or if something fundamental would need to change, ideally before I start rebasing.
👀
@griwes @jamestrew any updates on this?