Skip to content

Add a Scroll Bar #5304

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

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Add a Scroll Bar #5304

wants to merge 11 commits into from

Conversation

enricozb
Copy link
Contributor

@enricozb enricozb commented Apr 2, 2025

A scroll bar that also displays selection lines in the buffer.

This PR adds:

  • terminal_scroll_bar to ui_options, which controls whether the scroll bar should show.
  • ScrollBarGutter and ScrollBarHandle faces describing how to style the gutter and handle. The foreground of these faces will be the color of the characters used to display selection lines.

Screenshots

Here is a recording with the default styling and config
asciicast

Here is a recording with my personal styling and config:
asciicast

I've tested this with both non-remote and remote clients.

Issues

I am not the biggest fan of the implementation. The computation of Vector<LineCount> selection_lines happens on every draw call. This is needed to serialize these locations for RemoteUI and JsonUI. This is also computed regardless if the scrollbar is enabled, which is unfortunate.

I'm not sure what the appetite for adding something like this is, especially with my implementation. I saw there was a highlighter implementation (#2597) but did not honestly consider it. If we feel like this is best done as a highlighter I can investigate that approach as well.

@Delapouite
Copy link
Contributor

Thanks! I appreciate that it does more than a "regular" scroll bar by adding visual markers for selections.

Of course some other text-editors/IDE offer much more with other kinds of markers such as git added/changed/removed lines, or info/warning/error diagnostics. But then you end up with a "3 lanes" scroll bars with a lot of infos which can be confusing. "There's a red mark at the beginning of the buffer : is it red because of git removed lined or red because it's an LSP error?"

So here, I think you strike the right balance by only focusing on one of the core principle of Kakoune which is its multi-selections capabilities.

enricozb added 2 commits April 2, 2025 15:12
unfortunately this method causes the scrollbar to potentially have a
non-constant height.

however this method has the nice property of scrolling the scrollbar past the
end of the buffer, just like we do with padding.
@Screwtapello
Copy link
Contributor

Does doc/json_ui.asciidoc need to be updated with the changes to json_ui.cc?

@enricozb
Copy link
Contributor Author

enricozb commented Apr 2, 2025

While looking over the code, I think this would be best organized as a new method of UserInterface. Otherwise I'm kind of cluttering the arguments to UserInterface::draw.

when the wrap highlighter is added, the scroll bar size will reflect how
many line numbers are visible. if a wrapped line is effectively taking
up a lot of the vertical space in the window, the scroll bar handle will
be smaller to reflect that the user is viewing a smaller portion of the
file relative to the number of lines in the file.
@enricozb
Copy link
Contributor Author

enricozb commented Apr 5, 2025

I've improved the calculation of the size of the scroll bar handle. It works well with the wrap highlighter. It will decrease in size if the number of lines displayed to the user is small (due to a wrapped line taking up a lot of vertical space).

There is still a bug where when scrolling horizontally (which is not possible when the wrap highlighter is added), the scroll bar position is totally incorrect. This is because the display buffer range changes when scrolling horizontally. Still trying to fix this.

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.

3 participants