-
Notifications
You must be signed in to change notification settings - Fork 95
Conversation
charlespwd
commented
Jul 14, 2021
- Add Windows support to the language server (prep work for Shopify/theme-check-vscode#5)
- Add Windows executables
@@ -99,7 +99,7 @@ def text_document_uri(params) | |||
end | |||
|
|||
def path_from_uri(uri) | |||
uri&.sub('file://', '') | |||
uri&.sub('file://', '')&.sub('/c%3A', '') |
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.
don't you really want .chomp
? Or are you expecting \r in the middle of the string?
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.
It's the c:/
that I'm stripping. Not sure that applies?
# | ||
# Hours wasted: 8. | ||
eol = Gem.win_platform? ? "\n" : "\r\n" | ||
@out.write("Content-Length: #{response_body.bytesize}#{eol}") |
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.
for clarity, I'd suggest breaking out the line termination from the http header name/value pair. also, any concern that the bytesize will be null? If so, I'd want to add a safety here. (I'm guessing not since the response_body is fully buffered?
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.
Yeah the response_body
is a JSON.dump. I don't think that's possible unless you explicitly call send_response
without arguments.