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

internal: Clamp Position::character to line length 2/2 #18300

Merged
merged 1 commit into from
Oct 18, 2024

Conversation

krobelus
Copy link
Contributor

@krobelus krobelus commented Oct 15, 2024

Completes #18243

I don't think I have permissions to target this on the other PR, so we'll need to rebase manually

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 15, 2024
@Veykril
Copy link
Member

Veykril commented Oct 18, 2024

Okay line-index 0.1.2 is released now, so just needs a rebase

LSP says about Position::character

> If the character value is greater than the line length it defaults back to the line length.

but from_proto::offset() doesn't implement this.

A client might for example request code actions for a whole line by sending
Position::character=99999.  I don't think there is ever a reason (besides laziness) why the
client can't specify the line length instead but I guess we should not crash but follow protocol.

Technically it should be a warning, not an error but warning is not shown by default so keep
it at error I guess.

Fixes rust-lang#18240
@Veykril
Copy link
Member

Veykril commented Oct 18, 2024

Thanks!
@bors r+

@bors
Copy link
Collaborator

bors commented Oct 18, 2024

📌 Commit 94a4c3a has been approved by Veykril

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Oct 18, 2024

⌛ Testing commit 94a4c3a with merge 3ddfb0d...

@bors
Copy link
Collaborator

bors commented Oct 18, 2024

☀️ Test successful - checks-actions
Approved by: Veykril
Pushing 3ddfb0d to master...

@bors bors merged commit 3ddfb0d into rust-lang:master Oct 18, 2024
11 checks passed
@lnicola lnicola changed the title Clamp Position::character to line length 2/2 internal: Clamp Position::character to line length 2/2 Oct 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants