-
Notifications
You must be signed in to change notification settings - Fork 32
Add workspace symbols #106
Changes from 4 commits
25d171f
3c6f673
d46052b
10830b1
c3ba087
93704f5
0675f49
81ccecf
df9bc43
a0db775
719c354
c2543fd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
require "json" | ||
|
||
module Scry | ||
struct WorkspaceSymbolParams | ||
JSON.mapping({ | ||
query: String, | ||
}, true) | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
require "compiler/crystal/syntax" | ||
require "./protocol/workspace_symbol_params" | ||
|
||
module Scry | ||
class SymbolVisitor < Crystal::Visitor | ||
|
@@ -115,4 +116,26 @@ module Scry | |
ResponseMessage.new(@text_document.id, visitor.symbols) | ||
end | ||
end | ||
|
||
class WorkspaceSymbolProcessor | ||
def initialize(@msg_id : Int32 | String, @root_path : String, @query : String) | ||
@all_files = Dir.glob("#{root_path}/**/*.cr") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why doesn't this use the workspace, and find files based on prelude and requires? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
What do you mean? 😅 This will get all "workspace" files to analyze all "workspace" symbols There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh ok, so this symbol feature is just to search in your project (your own code), not everywhere (stdlib, other shards, etc) ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, just your code 😅 Although this should work on shards as well since There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok it's fine then. I think I'd even restrict to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @bew Well, I guess I like your idea of listing stdlib symbols, so, this feature will support listing symbols from:
And works very fast!!! 🎉 and stdlib is memoized ✨ |
||
end | ||
|
||
def run | ||
symbols = [] of SymbolInformation | ||
unless @query.empty? | ||
@all_files.each do |file| | ||
visitor = SymbolVisitor.new("file://#{file}") | ||
parser = Crystal::Parser.new(File.read(file)) | ||
parser.filename = file | ||
node = parser.parse | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what if there is a syntax error in one of the file? I think the exception needs to be rescued and the file skipped. Is it possible to send errors in addition to valid results? (say, here are the results for files X & Y, but there is an error with file Z) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess I can handle the error, but the response should still be managed by diagnostic provider I think I can return an empty array if syntax error is found There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looking at https://microsoft.github.io/language-server-protocol/specification#workspace-symbols-request-leftwards_arrow_with_hook you should be able to return null and an error in case an exception happens during the workspace symbol request. I think it could be possible to return a result & an error? but how the valid_result/error is handled would vastly depend on the language client implementation.. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, Actually we're managing that via I don't want to see a popup saying "LSP server has an error" every time I try to list symbols on files with syntax errors 😅 |
||
node.accept(visitor) | ||
symbols.concat visitor.symbols.select(&.name.match(Regex.new(@query))) | ||
end | ||
end | ||
|
||
ResponseMessage.new(@msg_id, symbols) | ||
end | ||
end | ||
end |
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.
Use regex here? Or add another spec for a regex query?
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.
Done! 👍
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.
A little
Use File.join
commit ? 😃