Skip to content

Set server pipes to binmode #366

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

Merged
merged 1 commit into from
May 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 5 additions & 1 deletion lib/ruby_lsp/ruby_lsp_rails/definition.rb
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,11 @@ def handle_route(node)
result = @client.route_location(T.must(node.message))
return unless result

file_path, line = result.fetch(:location).split(":")
*file_parts, line = result.fetch(:location).split(":")

# On Windows, file paths will look something like `C:/path/to/file.rb:123`. Only the last colon is the line
# number and all other parts compose the file path
file_path = file_parts.join(":")

@response_builder << Interface::Location.new(
uri: URI::Generic.from_path(path: file_path).to_s,
Expand Down
4 changes: 3 additions & 1 deletion lib/ruby_lsp/ruby_lsp_rails/runner_client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,9 @@ def initialize
if @wait_thread.alive?
$stderr.puts("Ruby LSP Rails is force killing the server")
sleep(0.5) # give the server a bit of time if we already issued a shutdown notification
Process.kill(T.must(Signal.list["TERM"]), @wait_thread.pid)

# Windows does not support the `TERM` signal, so we're forced to use `KILL` here
Process.kill(T.must(Signal.list["KILL"]), @wait_thread.pid)
end
end
end
Expand Down
2 changes: 2 additions & 0 deletions lib/ruby_lsp/ruby_lsp_rails/server.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ class Server
def initialize
$stdin.sync = true
$stdout.sync = true
$stdin.binmode
$stdout.binmode
@running = true
end

Expand Down
14 changes: 11 additions & 3 deletions test/ruby_lsp_rails/definition_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,10 @@ def baz; end

assert_equal(1, response.size)
dummy_root = File.expand_path("../dummy", __dir__)
assert_equal("file://#{dummy_root}/config/routes.rb", response[0].uri)
assert_equal(
URI::Generic.from_path(path: File.join(dummy_root, "config", "routes.rb")).to_s,
response[0].uri,
)
assert_equal(3, response[0].range.start.line)
assert_equal(3, response[0].range.end.line)
end
Expand All @@ -107,7 +110,10 @@ def baz; end

assert_equal(1, response.size)
dummy_root = File.expand_path("../dummy", __dir__)
assert_equal("file://#{dummy_root}/config/routes.rb", response[0].uri)
assert_equal(
URI::Generic.from_path(path: File.join(dummy_root, "config", "routes.rb")).to_s,
response[0].uri,
)
assert_equal(4, response[0].range.start.line)
assert_equal(4, response[0].range.end.line)
end
Expand All @@ -132,7 +138,9 @@ def generate_definitions_for_source(source, position)
params: { textDocument: { uri: uri }, position: position },
)

server.pop_response.response
result = server.pop_response
assert_instance_of(RubyLsp::Result, result)
result.response
end
end
end
Expand Down
14 changes: 6 additions & 8 deletions test/ruby_lsp_rails/runner_client_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,19 @@
require "test_helper"
require "ruby_lsp/ruby_lsp_rails/runner_client"

# tests are hanging in CI. https://github.com/Shopify/ruby-lsp-rails/issues/348
return if Gem.win_platform?

module RubyLsp
module Rails
class RunnerClientTest < ActiveSupport::TestCase
setup do
capture_subprocess_io do
@client = T.let(RunnerClient.new, RunnerClient)
end
@client = T.let(RunnerClient.new, RunnerClient)
end

teardown do
capture_subprocess_io { @client.shutdown }
assert_predicate @client, :stopped?
@client.shutdown

# On Windows, the server process sometimes takes a lot longer to shutdown and may end up getting force killed,
# which makes this assertion flaky
assert_predicate(@client, :stopped?) unless Gem.win_platform?
end

# These are integration tests which start the server. For the more fine-grained tests, see `server_test.rb`.
Expand Down
Loading