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

update x_offset calculation in Buffer::set_string_truncated #3839

Merged
merged 1 commit into from
Nov 17, 2022

Conversation

kristopherbullinger
Copy link
Contributor

Hello. First time contributor here.

SUMMARY:
With these changes, when truncate_start is true in a call to Buffer::set_string_truncated, the x_offset is now properly updated according to the (potentially truncated) content width.

Previously, the x_offset was never updated in this branch of code, which caused a layout error when rendering a Picker with items whose calculated labels contained multiple Spans. Since x_offset was not updated between calls to set_string_truncated, each span's content would render starting from the leftmost edge of the item's line, causing the content items to overlap each other. For example, in a menu item with two spans with content some_directory/ and 14.5 KB, it would render as 14.5 KBrectory/.

Also, I took the liberty of renaming a variable for added clarity.

when `truncate_start` is `true`, the `x_offset` is now properly updated
according to the width of the content or the truncated length.
@CBenoit CBenoit added S-waiting-on-review Status: Awaiting review from a maintainer. C-bug Category: This is a bug labels Sep 13, 2022
@kirawi kirawi added the A-helix-term Area: Helix term improvements label Sep 13, 2022
Copy link
Contributor

@AlexanderBrevig AlexanderBrevig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like a good thing to add a test case for.
I like the name change, it matches:

let content_width = content.width() as u16;

Copy link
Member

@archseer archseer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay on this! Looks good to me 👍🏻

@archseer archseer merged commit 7483c76 into helix-editor:master Nov 17, 2022
herkhinah pushed a commit to herkhinah/helix that referenced this pull request Dec 11, 2022
…editor#3839)

when `truncate_start` is `true`, the `x_offset` is now properly updated
according to the width of the content or the truncated length.
freqmod pushed a commit to freqmod/helix that referenced this pull request Feb 8, 2023
…editor#3839)

when `truncate_start` is `true`, the `x_offset` is now properly updated
according to the width of the content or the truncated length.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-helix-term Area: Helix term improvements C-bug Category: This is a bug S-waiting-on-review Status: Awaiting review from a maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants