- Notifications
You must be signed in to change notification settings - Fork226
fix Inlay hint insertion should consider imports/use qualified paths to auto import correctly#1576
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
kinto0 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.
awesome work! a few suggestions + it looks like there are some merge conflicts. it might also be worth coordinating with@jvansch1 over discord to make sure you're not stepping on each other's toes since he's editing the same file
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
| result:Some(serde_json::json!([ | ||
| { | ||
| "label":" -> tuple[Literal[1], Literal[2]]", | ||
| "label":" -> tuple[typing.Literal[1],typing.Literal[2]]", |
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.
cc@jvansch1 as you are also currently making changes to this
Uh oh!
There was an error while loading.Please reload this page.
kinto0 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.
looks like the insertion text no longer matches the type
| "label":" -> tuple[Literal[1], Literal[2]]", | ||
| "position":{"character":21,"line":6}, | ||
| "textEdits":[{ | ||
| "newText":" -> tuple[Literal[1], Literal[2]]", |
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.
| "newText":" -> tuple[Literal[1],Literal[2]]", | |
| "newText":" -> tuple[typing.Literal[1],typing.Literal[2]]", |
if weimport typing, we need to qualify these types with it
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.
I think this is still an issue
| "label":" -> tuple[Literal[1], Literal[2]]", | ||
| "position":{"character":21,"line":6}, | ||
| "textEdits":[{ | ||
| "newText":" -> tuple[Literal[1], Literal[2]]", |
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.
I think this is still an issue
This pull request has been automatically marked as stale because it has not had recent activity for more than 2 weeks. If you are still working on this this pull request, please add a comment or push new commits to keep it active. Otherwise, please unassign yourself and allow someone else to take over. Thank you for your contributions! |
fix part of#317
Type pretty-printing now tracks every module name it emits, so we can tell the client which imports a hint depends on; TypeDisplayContext stores the module set in a RefCell, exposes it via referenced_modules, and records each module.name combination as it formats the text
The LSP transaction layer wraps each hint in a new InlayHintWithEdits struct, renders annotations with fully qualified names, applies any local aliases (e.g., import typing as t), and bundles the necessary import edits by consulting the ImportTracker. This ensures that accepting a type hint also inserts import typing when the module hasn’t been brought into scope.
Downstream consumers now understand the richer hint payload: the server translates InlayHintWithEdits into multiple textEdits, the playground/WASM bindings expose the new label format, and the inlay-hint regression tests cover both standard and notebook scenarios to verify the extra import edits.