-
Notifications
You must be signed in to change notification settings - Fork 95
Add Snippet Completion + Document Links #223
Changes from all commits
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,43 @@ | ||
# frozen_string_literal: true | ||
|
||
module ThemeCheck | ||
module LanguageServer | ||
class RenderSnippetCompletionProvider < CompletionProvider | ||
def completions(content, cursor) | ||
return [] unless cursor_on_quoted_argument?(content, cursor) | ||
partial = snippet(content) || '' | ||
snippets | ||
.select { |x| x.starts_with?(partial) } | ||
.map { |x| snippet_to_completion(x) } | ||
end | ||
|
||
private | ||
|
||
def cursor_on_quoted_argument?(content, cursor) | ||
match = content.match(PARTIAL_RENDER) | ||
return false if match.nil? | ||
match.begin(:partial) <= cursor && cursor <= match.end(:partial) | ||
end | ||
|
||
def snippet(content) | ||
match = content.match(PARTIAL_RENDER) | ||
return if match.nil? | ||
match[:partial] | ||
end | ||
|
||
def snippets | ||
@storage | ||
.files | ||
.select { |x| x.include?('snippets/') } | ||
end | ||
|
||
def snippet_to_completion(file) | ||
{ | ||
label: File.basename(file, '.liquid'), | ||
kind: CompletionItemKinds::SNIPPET, | ||
detail: file, | ||
} | ||
end | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
# frozen_string_literal: true | ||
|
||
module ThemeCheck | ||
module LanguageServer | ||
PARTIAL_RENDER = %r{ | ||
\{\%\s*render\s+'(?<partial>[^']*)'| | ||
\{\%\s*render\s+"(?<partial>[^"]*)" | ||
}mix | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
# frozen_string_literal: true | ||
|
||
module ThemeCheck | ||
module LanguageServer | ||
class DocumentLinkEngine | ||
include PositionHelper | ||
include RegexHelpers | ||
|
||
def initialize(storage) | ||
@storage = storage | ||
end | ||
|
||
def document_links(uri) | ||
buffer = @storage.read(uri) | ||
matches(buffer, PARTIAL_RENDER).map do |match| | ||
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. Is this file specific 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. For now, yes. But we might want to add support for things that get piped to asset_url in the future. |
||
start_line, start_character = from_index_to_line_column( | ||
buffer, | ||
match.begin(:partial), | ||
) | ||
|
||
end_line, end_character = from_index_to_line_column( | ||
buffer, | ||
match.end(:partial) | ||
) | ||
|
||
{ | ||
target: link(match[:partial]), | ||
range: { | ||
start: { | ||
line: start_line, | ||
character: start_character, | ||
}, | ||
end: { | ||
line: end_line, | ||
character: end_character, | ||
}, | ||
}, | ||
} | ||
end | ||
end | ||
|
||
def link(partial) | ||
'file://' + @storage.path('snippets/' + partial + '.liquid') | ||
end | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,6 +8,7 @@ class Handler | |
triggerCharacters: ['.', '{{ ', '{% '], | ||
context: true, | ||
}, | ||
documentLinkProvider: true, | ||
textDocumentSync: { | ||
openClose: true, | ||
change: TextDocumentSyncKind::FULL, | ||
|
@@ -19,12 +20,13 @@ class Handler | |
def initialize(server) | ||
@server = server | ||
@previously_reported_files = Set.new | ||
@storage = InMemoryStorage.new | ||
@completion_engine = CompletionEngine.new(@storage) | ||
end | ||
|
||
def on_initialize(id, params) | ||
@root_path = params["rootPath"] | ||
@storage = in_memory_storage(@root_path) | ||
@completion_engine = CompletionEngine.new(@storage) | ||
@document_link_engine = DocumentLinkEngine.new(@storage) | ||
# https://microsoft.github.io/language-server-protocol/specifications/specification-current/#responseMessage | ||
send_response( | ||
id: id, | ||
|
@@ -59,6 +61,14 @@ def on_text_document_did_save(_id, params) | |
analyze_and_send_offenses(text_document_uri(params)) | ||
end | ||
|
||
def on_text_document_document_link(id, params) | ||
uri = text_document_uri(params) | ||
send_response( | ||
id: id, | ||
result: document_links(uri) | ||
) | ||
end | ||
|
||
def on_text_document_completion(id, params) | ||
uri = text_document_uri(params) | ||
line = params.dig('position', 'line') | ||
|
@@ -71,6 +81,23 @@ def on_text_document_completion(id, params) | |
|
||
private | ||
|
||
def in_memory_storage(root) | ||
config = ThemeCheck::Config.from_path(root) | ||
|
||
# Make a real FS to get the files from the snippets folder | ||
fs = ThemeCheck::FileSystemStorage.new( | ||
config.root, | ||
ignored_patterns: config.ignored_patterns | ||
) | ||
|
||
# Turn that into a hash of empty buffers | ||
files = fs.files | ||
.map { |fn| [fn, ""] } | ||
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. Just curious why you need to do this? 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. Because I need a list of snippets from the RenderSnippetCompletionProvider. The InMemoryStorage doesn't have anything in it and I didn't want to have both an InMemoryStorage instance for buffers and a FSStorage for files just so that the render snippet completion provider was able to list those files. 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 I see! Because that's what |
||
.to_h | ||
|
||
InMemoryStorage.new(files, root) | ||
end | ||
|
||
def text_document_uri(params) | ||
params.dig('textDocument', 'uri').sub('file://', '') | ||
end | ||
|
@@ -108,6 +135,10 @@ def completions(uri, line, col) | |
@completion_engine.completions(uri, line, col) | ||
end | ||
|
||
def document_links(uri) | ||
@document_link_engine.document_links(uri) | ||
end | ||
|
||
def send_diagnostics(offenses) | ||
reported_files = Set.new | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
# frozen_string_literal: true | ||
require "test_helper" | ||
|
||
module ThemeCheck | ||
module LanguageServer | ||
class DocumentLinkEngineTest < Minitest::Test | ||
include PositionHelper | ||
|
||
def test_makes_links_out_of_render_tags | ||
content = <<~LIQUID | ||
{% render 'a' %} | ||
{% render "b" %} | ||
LIQUID | ||
|
||
engine = make_engine( | ||
"snippets/a.liquid" => "", | ||
"snippets/b.liquid" => "", | ||
"templates/product.liquid" => content, | ||
) | ||
|
||
assert_links_include("a", content, engine.document_links("templates/product.liquid")) | ||
assert_links_include("b", content, engine.document_links("templates/product.liquid")) | ||
end | ||
|
||
def assert_links_include(needle, content, links) | ||
target = "file:///tmp/snippets/#{needle}.liquid" | ||
match = links.find { |x| x[:target] == target } | ||
|
||
refute_nil(match, "Should find a document_link with target == '#{target}'") | ||
|
||
assert_equal( | ||
from_index_to_line_column(content, content.index(needle)), | ||
[ | ||
match.dig(:range, :start, :line), | ||
match.dig(:range, :start, :character), | ||
], | ||
) | ||
|
||
assert_equal( | ||
from_index_to_line_column(content, content.index(needle) + 1), | ||
[ | ||
match.dig(:range, :end, :line), | ||
match.dig(:range, :end, :character), | ||
], | ||
) | ||
end | ||
|
||
private | ||
|
||
def make_engine(files) | ||
storage = InMemoryStorage.new(files, "/tmp") | ||
DocumentLinkEngine.new(storage) | ||
end | ||
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.
The reason I'm doing this is because I would otherwise have absolute paths in the InMemoryStorage. So we'd have duplicate entries for absolute paths and relative paths. I could also revert this and only have absolute paths.