Skip to content

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
@krobelus krobelus force-pushed the clamp-position-character-2 branch from e90dc6a to 94a4c3a Compare October 18, 2024 13:07
@Veykril
Copy link
Member

Veykril commented Oct 18, 2024

Thanks!
@bors r+

@bors
Copy link
Contributor

bors commented Oct 18, 2024

📌 Commit 94a4c3a has been approved by Veykril

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Oct 18, 2024

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

@bors
Copy link
Contributor

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