-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Implement Span::line() and Span::column() for proc-macro server #21405
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
Conversation
This comment has been minimized.
This comment has been minimized.
Shourya742
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 suggestion, rest of the changes looks good to me.
crates/load-cargo/src/lib.rs
Outdated
| let line_col = | ||
| line_index.try_line_col(range.range.start()).ok_or_else(|| ServerError { | ||
| message: format!("invalid offset {offset} for file {file_id}"), | ||
| io: None, | ||
| })?; | ||
| // proc_macro::Span line/column are 1-based | ||
| Ok(SubResponse::LineColumnResult { | ||
| line: line_col.line as u32 + 1, | ||
| column: line_col.col as u32 + 1, | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can return a default value instead of propagating an error, given how things are structured right now. If an error occurs in the callback, we end up interrupting the client request while the server continues to block waiting for a response. This is something we’ll improve, but since we’re still prototyping, returning a default value is the simplest and safest option for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
{1,1} then as in the previous implementation?
Add proper line/column resolution for proc-macro spans via a callback mechanism. Previously these methods returned hardcoded 1 values. The implementation adds: - SubRequest::LineColumn and SubResponse::LineColumnResult to the bidirectional protocol - ProcMacroClientInterface::line_column() method - Callback handling in load-cargo using LineIndex - Server implementation in RaSpanServer that uses the callback - a test for Span::line() and Span::column() in proc-macro server Add fn_like_span_line_column test proc-macro that exercises the new line/column API, and a corresponding test with a mock callback.
Add proper line/column resolution for proc-macro spans via a callback mechanism. Previously these methods returned hardcoded 1 values.
The implementation adds:
SubRequest::LineColumn and SubResponse::LineColumnResult to the bidirectional protocol
ProcMacroClientInterface::line_column() method
Callback handling in load-cargo using LineIndex
Server implementation in RaSpanServer that uses the callback
Test infrastructure and a test for LineColumn
This is in service of resolving an issue with Slint: slint-ui/slint#685
The Slint side now implement this also as per:
slint-ui/slint@13ce62d
Owing to my limited experience with Rust (and rust-analyzer) this was created with help from Claude. I have attempted to steer and review to the best of my ability and attempted sufficient test coverage.