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

fix(lsp): don't normalize urls in cache command params #22182

Merged
merged 1 commit into from Jan 30, 2024

Conversation

nayeemrmn
Copy link
Collaborator

Fixes #22122. We shouldn't do this normalization because the params to the cache command come from the server's quick-fixes and are never denormalized to client URLs in the first place. It was causing a faulty memoization in the URL mapper.

BTW the fix is a one line change removing the normalize_url() call from https://github.com/denoland/deno/blob/v1.40.2/cli/lsp/language_server.rs#L3723. But I cleaned up the param types across the cache command helpers to make it clear that these are actual module specifiers, not client URLs. Also removed some old custom requests that were moved to commands.

Copy link
Member

@dsherret dsherret left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

self.task_queue.queue_task(Box::new(|ls: LanguageServer| {
spawn(async move {
if let Err(err) =
ls.cache_request(Some(json!(cache_params))).await
ls.cache_request(specifiers, referrer).await
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1... kind of crazy we were doing that 😅

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.

Caching then "go to definition" is broken until lsp restart
2 participants