Skip to content

Reapply "[Feature #6012] Extend source_location for end position #15580

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.

Sign up for GitHub

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

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

Reapply "[Feature #6012] Extend source_location for end position #15580

eregon wants to merge 3 commits into from
+98 −60

Conversation

@eregon
Copy link
Member

@eregon eregon commented Dec 16, 2025 •
edited
Loading

Also:

  • [Bug #21783] Fix documentation of {Method,UnboundMethod,Proc}#source_location
  • [Bug #21784] Fix the Proc#source_location start_column for stabby lambdas

So this PR fixes all known issues about {Method,UnboundMethod,Proc}#source_location.

@eregon
Reapply "[Feature ruby#6012] Extend source_location for end position
8ab6a1d 
* This reverts commit 065c48c.
* This functionality is very valuable and has already taken 14 years
  to agree on the API.
* Let's just document it's byte columns (in the next commit).
* See https://bugs.ruby-lang.org/issues/21783#note-9
@eregon eregon requested review from kddnewton, matz and nobu December 16, 2025 09:39
@eregon eregon mentioned this pull request Dec 16, 2025
Closed
nobu
nobu reviewed Dec 16, 2025
View reviewed changes
Copy link
Member

@nobu nobu left a comment

Choose a reason for hiding this comment

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

I think we should avoid the word “column” in the documents, because of the ambiguities.

@eregon
Copy link
Member Author

eregon commented Dec 16, 2025 •
edited
Loading

Good point, especially since column might imply it starts at 1.
Do you have any suggestion?
The current text in 2d579f8 is:

(3) the column where the definition starts, in number of bytes from the start of the line

So maybe:

(3) the byte index where the definition starts, counted from the start of the line

(using matz's words from https://bugs.ruby-lang.org/issues/21783#note-8)
or

(3) the position where the definition starts, in number of bytes from the start of the line

?
Let me know what you think is better, happy to update.

@eregon
[Bug #21783] Fix documentation of {Method,UnboundMethod,Proc}#source_…
85d395c 
…location
@eregon
[Bug #21784] Fix the Proc#source_location start_column for stabby lam…
a1c18d0 
…bdas

* Consistent with plain `blocks` and `for` blocks and methods
  where the source_location covers their entire definition.
* Matches the documentation which mentions
  "where the definition starts/ends".
* Partially reverts d357d50
  which was a workaround to be compatible with parse.y.
@eregon eregon force-pushed the source_location_byte_columns_for_4_0 branch from 7ac7008 to a1c18d0 Compare December 16, 2025 13:35
@eregon
Copy link
Member Author

eregon commented Dec 16, 2025 •
edited
Loading

I have updated to this one for now:

(3) the position where the definition starts, in number of bytes from the start of the line

because "byte index" sounds to me rather confusing in this context.

@kddnewton
Copy link
Contributor

kddnewton commented Dec 18, 2025

I'm fine with the code in here, pending approval from whomever makes the final decision.

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

Reviewers

@nobu nobu nobu left review comments

@matz matz Awaiting requested review from matz

@kddnewton kddnewton Awaiting requested review from kddnewton

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@eregon @kddnewton @nobu