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 :LspDenoDefinition to handle correctly #782

Merged
merged 1 commit into from
Nov 26, 2024

Conversation

heavenshell
Copy link
Contributor

@heavenshell heavenshell commented Nov 24, 2024

Run :LspDenoDefinition twice at built-in keyword/function such as console, the result of the second run is not processed correctly.

Steps to reproduce

  1. Run deno init
  2. Open main.ts and move cursor to console keyword
  3. type :LspDenoDefinition and jump to definition
  4. type :bdelete
  5. move cursor to console keyword again
  6. type :LspDenoDefinition and not jump to definition

Note: I'm using latest deno(v2.1.1) but this may happen In old version such as 1.38.1.

Log

1st try

Sun Nov 24 10:41:20 2024:["--->",1,"deno",{"method":"textDocument/definition","on_notification":"---funcref---","params":{"textDocument":{"uri":"file:///tmp/deno_sampe/main.ts"},"position":{"character":3,"line":6}}}] 
Sun Nov 24 10:41:20 2024:["<---",1,"deno",{"response":{"id":3,"jsonrpc":"2.0","result":[{"targetSelectionRange":{"end":{"character":19,"line":461},"start":{"character":12,"line":461}},"targetRange":{"end":{"character":29,"line":461},"start":{"character":0,"line":461}},"targetUri":"deno:/asset/lib.deno.shared_globals.d.ts"}]},"request":{"id":3,"jsonrpc":"2.0","method":"textDocument/definition","params":{"textDocument":{"uri":"file:///tmp/deno_sampe/main.ts"},"position":{"character":3,"line":6}}}}]  

2nd try

Sun Nov 24 10:41:24 2024:["--->",1,"deno",{"method":"textDocument/definition","on_notification":"---funcref---","params":{"textDocument":{"uri":"file:///tmp/deno_sampe/main.ts"},"position":{"character":3,"line":6}}}]
Sun Nov 24 10:41:24 2024:["<---",1,"deno",{"response":{"id":9,"jsonrpc":"2.0","result":[{"targetSelectionRange":{"end":{"character":19,"line":461},"start":{"character":12,"line":461}},"targetRange":{"end":{"character":29,"line":461},"start":{"character":0,"line":461}},"targetUri":"file:///tmp/deno_sampe/deno%3A/asset/lib.deno.shared_globals.d.ts"}]},"request":{"id":9,"jsonrpc":"2.0","method":"textDocument/definition","params":{"textDocument":{"uri":"file:///tmp/deno_sampe/main.ts"},"position":{"character":3,"line":6}}}}]

I think this is because did open sent deno's virtual text document's uri to language server.

Sun Nov 24 10:41:20 2024:["--->",1,"deno",{"method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///tmp/deno_sampe/deno%3A/asset/lib.deno.shared_globals.d.ts","version":1,"languageId":"typescript","text":"\n"}}}]

It seems to be correct to fix the didOpen part, but it is currently too difficult, so I created this workaround.

@mattn
Could you review this. Any advices would be greatly appreciated 🙏

Before:

Kapture.2024-11-25.at.00.23.00.mp4

After:

Kapture.2024-11-25.at.00.24.25.mp4

Thank you.

Run `:LspDenoDefinition` twice at built-in function, the result of the second
run is not processed correctly
@mattn mattn merged commit a36baad into mattn:master Nov 26, 2024
14 checks passed
@mattn
Copy link
Owner

mattn commented Nov 26, 2024

Thank you!

@heavenshell heavenshell deleted the topic/deno-definition branch November 26, 2024 11:16
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.

2 participants