gpui: Unify the index_for_x methods #42162
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cla-bot
bot
added
the
cla-signed
mikayla-maki
force-pushed
the
replace-index-for-x
branch
from
fb03290 to
e5b0484
Compare
mikayla-maki
commented
Nov 7, 2025
| let fragment_end_x = fragment_start_x + shaped_line.width; | ||
| if x < fragment_end_x { | ||
| return Some( | ||
| fragment_start_index + shaped_line.index_for_x(x - fragment_start_x)?, |
Member
Author
There was a problem hiding this comment.
From what I can tell, this is the only line of code that could be effected by removing the "overshoot returns None" part of the API. However, I believe this case is impossible to reach due to the x < fragment_end_x check above it, and so we can safely move forward with this API change.
SomeoneToIgnore
approved these changes
Nov 7, 2025
tomatitito
pushed a commit
to tomatitito/zed
that referenced
this pull request
Nov 7, 2025
Supersedes zed-industries#39910 At some point, these two (`index_for_x` and `closest_index_for_x`) methods where separated out and some code paths used one, while other code paths took the other. That said, their behavior is almost identical: - `index_for_x` computes the index behind the pixel offset, and returns `None` if there's an overshoot - `closest_index_for_x` computes the nearest index to the pixel offset, taking into account whether the offset is over halfway through or not. If there's an overshoot, it returns the length of the line. Given these two behaviors, `closest_index_for_x` seems to be a more useful API than `index_for_x`, and indeed the display map and other core editor features use it extensively. So this PR is an experiment in simply replacing one behavior with the other. Release Notes: - Improved the accuracy of mouse selections in Markdown
SomeoneToIgnore
added a commit
that referenced
this pull request
Nov 12, 2025
This reverts commit 082b80e.
SomeoneToIgnore
added a commit
that referenced
this pull request
Nov 12, 2025
11happy
pushed a commit
to 11happy/zed
that referenced
this pull request
Dec 1, 2025
Supersedes zed-industries#39910 At some point, these two (`index_for_x` and `closest_index_for_x`) methods where separated out and some code paths used one, while other code paths took the other. That said, their behavior is almost identical: - `index_for_x` computes the index behind the pixel offset, and returns `None` if there's an overshoot - `closest_index_for_x` computes the nearest index to the pixel offset, taking into account whether the offset is over halfway through or not. If there's an overshoot, it returns the length of the line. Given these two behaviors, `closest_index_for_x` seems to be a more useful API than `index_for_x`, and indeed the display map and other core editor features use it extensively. So this PR is an experiment in simply replacing one behavior with the other. Release Notes: - Improved the accuracy of mouse selections in Markdown
11happy
pushed a commit
to 11happy/zed
that referenced
this pull request
Dec 1, 2025
…ed-industries#42505) This reverts commit 082b80e. This broke clicking, e.g. in snippets like ```rs let x = vec![ 1, 2, // 3, ]; ``` clicking between `2` and `,` is quite off now. Release Notes: - N/A
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Supersedes #39910
At some point, these two (
index_for_xandclosest_index_for_x) methods where separated out and some code paths used one, while other code paths took the other. That said, their behavior is almost identical:index_for_xcomputes the index behind the pixel offset, and returnsNoneif there's an overshootclosest_index_for_xcomputes the nearest index to the pixel offset, taking into account whether the offset is over halfway through or not. If there's an overshoot, it returns the length of the line.Given these two behaviors,
closest_index_for_xseems to be a more useful API thanindex_for_x, and indeed the display map and other core editor features use it extensively. So this PR is an experiment in simply replacing one behavior with the other.Release Notes: