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

Position of RBS::Location should be byte length #1814

Open
ksss opened this issue May 1, 2024 · 3 comments
Open

Position of RBS::Location should be byte length #1814

ksss opened this issue May 1, 2024 · 3 comments

Comments

@ksss
Copy link
Collaborator

ksss commented May 1, 2024

I'm developing a tool using RBS.
And I'm trying to load RBS files using RBS::MethodType#location and display them with colors.

During development, a problem occurred where the display is off by one character only in core/math.rbs.

In core/math.rbs, the character π is used in the documentation, which is a 2 byte character.

Upon investigation, it appears that the positions in Location are recorded not in byte size but in the number of UTF-8 characters.

# Whether the comment is `π` or `p`, the class location has not changed.

::RBS::Parser.parse_signature("#π\nclass Foo\nend")[2][0].location.start_pos
#=> 3
::RBS::Parser.parse_signature("#p\nclass Foo\nend")[2][0].location.start_pos
#=> 3

Recording the position in bytesize would make it more convenient for reading with methods like IO#read, so I believe it should be recorded in bytesize.

What do you think?

@soutaro
Copy link
Member

soutaro commented May 1, 2024

I think we need both of character position and byte position, based on the use cases.
Storing byte positions and calculate character position when we need, like Prism, might be a good way.

@pocke
Copy link
Member

pocke commented May 1, 2024

I'm concerned about that it introduces performance degradation if RBS calculates a character position from a byte position. Currently, RBS uses only character positions at the Ruby level; therefore, it will need the calculations for every RBS::Location usage.

@ksss
Copy link
Collaborator Author

ksss commented May 1, 2024

We can use a workaround to temporarily avoid the issue by utilizing methods like start_loc and end_loc to ignore the effects of comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants