Skip to content

Allow CLI flags for lsp #99

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jun 21, 2022
Merged

Allow CLI flags for lsp #99

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ac16838
Respond to LSP Shutdown command to prevent VS Code deadlocks
xeger Jun 19, 2022
480f095
Allow CLI flags for lsp
xeger Jun 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 15 additions & 15 deletions lib/syntax_tree/cli.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ def run(item)
#{Color.bold("stree help")}
Display this help message

#{Color.bold("stree lsp")}
#{Color.bold("stree lsp [OPTIONS]")}
Run syntax tree in language server mode

#{Color.bold("stree version")}
Expand All @@ -239,6 +239,20 @@ class << self
def run(argv)
name, *arguments = argv

# If there are any plugins specified on the command line, then load them
# by requiring them here. We do this by transforming something like
#
# stree format --plugins=haml template.haml
#
# into
#
# require "syntax_tree/haml"
#
if arguments.first&.start_with?("--plugins=")
plugins = arguments.shift[/^--plugins=(.*)$/, 1]
plugins.split(",").each { |plugin| require "syntax_tree/#{plugin}" }
Copy link
Contributor Author

@xeger xeger Jun 19, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Doing a self review, it strikes me that it's not obvious how one would register a third-party plugin (that isn't part of the syntax_tree gem).

I suppose another gem could expose a lib/syntax_tree/foo source, which might work in a Bundler setting (where the gem is pre-activated and its lib is added to the load paths).

When using system gems, I'm not sure this "delegated lib subdir" idea would work, as I vaguely recall some magic with require'ing a gem name and auto-activating it which might foil "deep require" of nonstandard-named lib subdirs... it's been too long since I've been without Bundler!

Perhaps the CLI needs an -r option to ensure gems are activated .. or perhaps an executable config file that can do whatever and configure syntax_tree is a better fit? 🤔

Anyhow: my comment applies to all uses of this option, lsp or no; not particularly relevant to this PR.

Copy link
Member

Choose a reason for hiding this comment

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

I tried to be explicit about this in the README (https://github.com/ruby-syntax-tree/syntax_tree#configuration). It should be fine to add a syntax_tree directory under a path that is already in the require path. There are a lot of gems that do this (for example any Rails plugin that adds generators).

If folks need other gems activated that they for some reason can't require in their own plugins, I'm going to say they should use RUBYOPT instead. This isn't meant to be a whole replacement for the require system, just a small convenience option.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I hadn't considered RUBYOPT that ought to be sufficient for anyone's needs.

end

case name
when "help"
puts HELP
Expand Down Expand Up @@ -282,20 +296,6 @@ def run(argv)
return 1
end

# If there are any plugins specified on the command line, then load them
# by requiring them here. We do this by transforming something like
#
# stree format --plugins=haml template.haml
#
# into
#
# require "syntax_tree/haml"
#
if arguments.first&.start_with?("--plugins=")
plugins = arguments.shift[/^--plugins=(.*)$/, 1]
plugins.split(",").each { |plugin| require "syntax_tree/#{plugin}" }
end

# We're going to build up a queue of items to process.
queue = Queue.new

Expand Down
3 changes: 2 additions & 1 deletion lib/syntax_tree/language_server.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,9 @@ def run
write(id: id, result: { capabilities: capabilities })
in method: "initialized"
# ignored
in method: "shutdown"
in method: "shutdown" # tolerate missing ID to be a good citizen
store.clear
write(id: request[:id], result: {})
return
in {
method: "textDocument/didChange",
Expand Down
36 changes: 26 additions & 10 deletions test/language_server_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ def to_hash
end
end

class Shutdown
class Shutdown < Struct.new(:id)
def to_hash
{ method: "shutdown" }
{ method: "shutdown", id: id }
end
end

Expand Down Expand Up @@ -107,13 +107,14 @@ def test_formatting
TextDocumentDidChange.new("file:///path/to/file.rb", "class Bar; end"),
TextDocumentFormatting.new(2, "file:///path/to/file.rb"),
TextDocumentDidClose.new("file:///path/to/file.rb"),
Shutdown.new
Shutdown.new(3)
]

case run_server(messages)
in [
{ id: 1, result: { capabilities: Hash } },
{ id: 2, result: [{ newText: new_text }] }
{ id: 2, result: [{ newText: new_text }] },
{ id: 3, result: {} }
]
assert_equal("class Bar\nend\n", new_text)
end
Expand All @@ -129,13 +130,14 @@ def test_inlay_hints
end
RUBY
TextDocumentInlayHints.new(2, "file:///path/to/file.rb"),
Shutdown.new
Shutdown.new(3)
]

case run_server(messages)
in [
{ id: 1, result: { capabilities: Hash } },
{ id: 2, result: { before:, after: } }
{ id: 2, result: { before:, after: } },
{ id: 3, result: {} }
]
assert_equal(1, before.length)
assert_equal(2, after.length)
Expand All @@ -147,11 +149,15 @@ def test_visualizing
Initialize.new(1),
TextDocumentDidOpen.new("file:///path/to/file.rb", "1 + 2"),
SyntaxTreeVisualizing.new(2, "file:///path/to/file.rb"),
Shutdown.new
Shutdown.new(3)
]

case run_server(messages)
in [{ id: 1, result: { capabilities: Hash } }, { id: 2, result: }]
in [
{ id: 1, result: { capabilities: Hash } },
{ id: 2, result: },
{ id: 3, result: {} }
]
assert_equal(
"(program (statements ((binary (int \"1\") + (int \"2\")))))\n",
result
Expand All @@ -167,13 +173,14 @@ def test_reading_file
messages = [
Initialize.new(1),
TextDocumentFormatting.new(2, "file://#{file.path}"),
Shutdown.new
Shutdown.new(3)
]

case run_server(messages)
in [
{ id: 1, result: { capabilities: Hash } },
{ id: 2, result: [{ newText: new_text }] }
{ id: 2, result: [{ newText: new_text }] },
{ id: 3, result: {} }
]
assert_equal("class Foo\nend\n", new_text)
end
Expand All @@ -186,6 +193,15 @@ def test_bogus_request
end
end

def test_clean_shutdown
messages = [Initialize.new(1), Shutdown.new(2)]

case run_server(messages)
in [{ id: 1, result: { capabilities: Hash } }, { id: 2, result: {} }]
assert_equal(true, true)
end
end

private

def write(content)
Expand Down