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.

Sign up for GitHub

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

Jump to bottom

Only highlight inner area of view with cursorline #2935

Closed
wants to merge 2 commits into from
Closed

Only highlight inner area of view with cursorline #2935

wants to merge 2 commits into from

Conversation

A-Walrus
Copy link
Contributor

@A-Walrus A-Walrus commented Jul 1, 2022

Fixes #2912

archseer
archseer requested changes Jul 1, 2022
View reviewed changes
helix-term/src/ui/editor.rs Outdated Show resolved Hide resolved
@A-Walrus A-Walrus requested a review from archseer July 1, 2022 15:17
@archseer
Copy link
Member

I actually like the current behavior though.

@ChrHorn
Copy link
Contributor

ChrHorn commented Jul 18, 2022

I actually like the current behavior though.

With this PR it is still possible to set ui.linenr.selected background color same as cursorline to achieve the current behavior (minus the diagnostics and the gap).

@archseer
Copy link
Member

Yeah it's not the same, it won't highlight all the gutters.

@ChrHorn
Copy link
Contributor

ChrHorn commented Jul 23, 2022 •
edited
Loading

Yeah it's not the same, it won't highlight all the gutters.

I see. What if we just use ui.linenr(.selected) for the whole gutter and rename it to ui.gutter(.selected) (ui.gutter is already used but not documented). Think this looks better in general and would allow both styles for the cursorline.

Could also use a more fine-grained approach where every selected gutter component can be themed individually, but I'm not sure if this is really needed.

@archseer
Copy link
Member

archseer commented Aug 2, 2022

ui.gutter.selected is a good idea, but I'd also keep ui.linenr.selected so that only the line number foreground can be changed.

@ChrHorn ChrHorn mentioned this pull request Aug 2, 2022
Merged
@kirawi kirawi added C-bug Category: This is a bug A-helix-term Area: Helix term improvements A-theme Area: Theme and appearence related S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 13, 2022
@kirawi kirawi added S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 24, 2022
@pascalkuthe
Copy link
Member

closing this one out as stale, the entire rendering code has been replaced (and will see further large changes) so it would be easier to start over with this one. Thank ou for contributing!

@the-mikedavis the-mikedavis mentioned this pull request Nov 6, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sudormrfbin sudormrfbin sudormrfbin approved these changes

@archseer archseer Awaiting requested review from archseer

Assignees
No one assigned
Labels
A-helix-term Area: Helix term improvements A-theme Area: Theme and appearence related C-bug Category: This is a bug S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Restrict cursorline to editing area
6 participants
@A-Walrus @archseer @ChrHorn @pascalkuthe @sudormrfbin @kirawi