telescope.nvim
telescope.nvim copied to clipboard
nvim-telescope
Use extmarks for highlighting and carets
Description
Pulled out as a subset of the changes from #1491
This PR uses extmarks and virtual text instead of typical highlighting and text replacement to consolidate the highlight/rendering logic to highlights.lua. This should give us more flexibility in the future when we want to improve rendering of telescope.
Type of change
Please delete options that are not relevant.
- Internal Changes
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
- [x] All the tests in
lua/tests/automated/
Configuration:
- Neovim version (nvim --version): NVIM v0.7.2
- Operating system and version: macOS 12.3.1
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
- [x] I have made corresponding changes to the documentation (lua annotations)
Thank you very much for pulling out these changes :)
I'll review and test it tomorrow evening. I can't make it before
No worries, take your time. I’m here to answer any questions
Sent from my iPhone
On Jul 26, 2022, at 12:39 PM, Simon Hauser @.***> wrote:
Thank you very much for pulling out these changes :)
I'll review and test it tomorrow evening. I can't make it before
— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.
@codegangsta you can run stylua over a direction stylua . to autoformat (not sure if you were doing that alreayd)
@tjdevries Yeah I thought I has stylua installed already but it looks like I didn't. Should be all good now
Overall LGTM, just a couple of nits :laughing: Thanks again for pulling it out :)
One thing i noticed while testing is that on multiselect we change the highlighting for the icon, which we currently dont do. I think priority is causing this and i am currently thinking if DISPLAY should have a higher priority compared to SELECTION.
CC @fdschmidt93
Oh good catch on the highlight. If we want to keep it compatible the priority fix should be easy enough. If we decide later we want to change it to highlight the multi-select cursor we can do that in a followup PR.
Yeah can you update the priorities, and fix the other things. You also need to rebase :grimacing:
Once this is fixed, @fdschmidt93 can you retest and merge. I am leaving this evening for a 10 day vacation and will not take a laptop and actually going to uninstall github from my phone, so yeah thanks :)
@Conni2461 and @fdschmidt93 fixed the feedback in this PR. LMK if there are any outstanding issues
oh right, tests will fail because we no longer overwrite the text in the line -- you can just update the tests to pass @codegangsta they all seem like failures that are OK to just be shifted to new results
Whoops. Missed that one as it looks like that test was disabled on mac. All fixed now @tjdevries