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

Use position zero for start of string in position at #7

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

Conversation

JediLuke
Copy link

@JediLuke JediLuke commented Oct 16, 2022

I think this is a bug, let me know if it was actually intentional... right now position_at counts position zero as being the first character, so getting a string width at position zero returns the width of 1 character. Position 1 will show the width of 2 characters, etc

iex(10)> s
"AAA"
iex(5)> FontMetrics.position_at(s, 0, 12, bm)
{8.700000000000001, 0}
iex(6)> FontMetrics.position_at(s, 1, 12, bm)
{17.400000000000002, 0}
iex(7)> FontMetrics.position_at(s, 2, 12, bm)
{26.1, 0}
iex(8)> FontMetrics.position_at(s, 2, 12, bm)
{26.1, 0}
iex(9)> FontMetrics.position_at(s, 3, 12, bm)
{26.1, 0}
iex(11)> FontMetrics.position_at(s, 10, 12, bm)
{26.1, 0}

I think position 0 should return a length of zero, to make it more consistent with how String.length/1 works:

iex(1)> String.length("")
0
iex(2)> String.length("1")
1

The way I discovered this was when working on positioning a cursor, I was using String.length/1 to figure out where to place my cursor - but due to the way position_at currently works (over-estimating the position by one character) moving my cursor back 1 character wasn't achieving the desired effect. With these changes now my cursor works well. Note though that this might end up affecting other people's code, including the existing TextBox component (I haven't checked this I'm just guessing) if they work by depending on the current behaviour

@crertel
Copy link

crertel commented Oct 16, 2022

🤔

What's the behavior of the existing API for a position_at of an empty string and position 0?

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.

2 participants