Skip to content

Include trailing colon in RBS keyword parameter offset#694

Closed
soutaro wants to merge 1 commit intomainfrom
soutaro-rbs-keyword-offset
Closed

Include trailing colon in RBS keyword parameter offset#694
soutaro wants to merge 1 commit intomainfrom
soutaro-rbs-keyword-offset

Conversation

@soutaro
Copy link
Copy Markdown
Contributor

@soutaro soutaro commented Mar 26, 2026

Summary

  • Align RBS keyword parameter offsets with Ruby indexer behavior by including the trailing colon (e.g. name: instead of name)
  • Add Offset::extend_end helper to adjust byte ranges

Extracted from #686.

Test plan

  • Existing tests updated to assert the new offset behavior

🤖 Generated with Claude Code

Align RBS keyword parameter offsets with Ruby indexer behavior.
The RBS indexer now includes the trailing colon in the offset for
required and optional keyword parameters (e.g. `name:` instead of `name`).

Adds `Offset::extend_end` helper to adjust byte ranges.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@soutaro soutaro requested a review from a team as a code owner March 26, 2026 02:34
@soutaro soutaro self-assigned this Mar 26, 2026
@vinistock
Copy link
Copy Markdown
Member

Wait, I didn't realize we were actually including the : as part of the name. I think this is a mistake of the Ruby indexer, RBS was doing it right.

Or do you see any value in including the comma? I think that would actually make things more difficult for rename or even diagnostics (since they would need to adjust the location by 1).

@soutaro
Copy link
Copy Markdown
Contributor Author

soutaro commented Mar 27, 2026

@vinistock 🤣

In fact I first tried to fix the RubyIndexer, but I assumed there was a reason to include the comma. Will fix the RubyIndexer then. 💪

@soutaro soutaro closed this Mar 27, 2026
@soutaro soutaro deleted the soutaro-rbs-keyword-offset branch March 27, 2026 05:56
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