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

fix: stacking highlights in buffer previewer #1294

Merged

Conversation

fdschmidt93
Copy link
Member

@fdschmidt93 fdschmidt93 commented Sep 28, 2021

Closes #1213

This is a slightly (but totally ok) hacky solution because we'd otherwise need hl_mode to affect hl_group in nvim_buf_set_extmark.

@lcrockett does it now work as expected for you? For me it seems to work as expected with your minimal config.

@fdschmidt93 fdschmidt93 changed the title fix: stacking highlights fix: stacking highlights in buffer previewer Sep 28, 2021
@Conni2461
Copy link
Member

Makes sense to me. You can merge after running stylua 😆

Offtopic: Did you see #1300 (comment) maybe we need to move away from using the same buffers, which kinda sucks but mru is more important than syntax hightlighting for the terminal buffers. Idk what to do because i kinda dont wanna move away from it 😆 It took you so long to make it work

@fdschmidt93 fdschmidt93 merged commit ec48777 into nvim-telescope:master Oct 4, 2021
@fdschmidt93
Copy link
Member Author

fdschmidt93 commented Oct 4, 2021

Sorry, deleted / turned off all telescope notifications the past days as I'm quite busy atm and missed style failure 😅

I looked at #1300 only earlier today and wanted to revisit it some other time hoping to find some good solution (agree that pretty sure there's not).

Let's just move back and make "actual buffers" previewer opt-in while documenting this issue. I fully agree mru is more important than syntax highlighting, though a to me surprising amount of users was "upvoting" #714 Any future issues with the "actual buffers" previewer are then left to fix to its users, because I'm kind of done with it. Further, I'm sure you're aware, but we then have to revert some stuff with the special buffers for the "cloning buffers" previewer. I'll sit on the train Wednesday morning in all likelihood; I'll do my very best (honestly, no hard promises because it's just sucking motivation) to prepare the PR then if you agree with this plan.

@fdschmidt93 fdschmidt93 deleted the fix/stacking-buf-prev-hl branch October 4, 2021 20:49
@Conni2461
Copy link
Member

No worries :) I also am currently not motivated to do anything. I dont like the idea of doing it opt-in i would just do more opinionated things rather than make everyone happy. Because this just leads to more bugs and we will never make everyone happy. People can still overwrite it by passing in a different function pointer (to previewer = ...).

I've been thinking quite a lot where we should go with telescope and ^^^ is the reason. I havent found a solution for the issues we currently face. #1228 is obv related to this.

@lcrockett
Copy link

Disregarding the current discussion here; @fdschmidt93 this PR fixes #1213 indeed and the original behaviour has returned with it. Not too sure if based on the discussion here this might change or not in the future. However, cheers on this and other efforts regarding nvim-telescope !

@fdschmidt93
Copy link
Member Author

I dont like the idea of doing it opt-in i would just do more opinionated things rather than make everyone happy. Because this just leads to more bugs and we will never make everyone happy.

Yeah, you're totally right. Probably then just off to the wiki with it or something eventually.

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

Successfully merging this pull request may close these issues.

Buffers picker :: Odd colorization of highlighted lines
3 participants