Improvements for rustc completer #1
Conversation
| @@ -82,7 +74,7 @@ def _GetResponse( self, handler, request_data = {} ): | |||
| were found. | |||
| """ | |||
| self._logger.info('RustCompleter._GetResponse') | |||
| target = urlparse.urljoin( self._racerd_host, handler ) | |||
| target = 'http://localhost:' + str(self._racerd_port) + str(handler) | |||
There was a problem hiding this comment.
I don't like this at all. What was the reason of this change?
There was a problem hiding this comment.
@vheon I removed self._racerd_host completely, it was generated from output of racerd command. It's using http://localhost prefix and port number from utils.GetUnusedLocalhostPort() now.
There was a problem hiding this comment.
but what is wrong with urlparse.urljoin?
There was a problem hiding this comment.
Ahh I completely missed that, I believe it wasn't necessary to use urlparse.urljoin for this but I will add it again.
There was a problem hiding this comment.
There is a race condition with utils.GetUnusedLocalhostPort. Is there a problem with reading the initial output from racerd?
There was a problem hiding this comment.
We are aware of the race condition with utils.GetUnusedLocalhostPort but we use it with:
- YouCompleteMe spawning ycmd
- Spawing OmnisharpServer
- Spawning JediHTTP (in the future 😝 )
- Spawning Tern.js (there is a PR for Tern.js support)
The problem with reading the initial output from racerd is that for doing that you should wait for the server to start printing stuff on stdout instead of telling the server to start and move on. Plus it makes the code much simpler.
|
I don't know if @jwilm will merge this but I made a couple of comments :) |
* Add _FindRacerdBinary and _FindRustSrcPath methods to get racerd binary path and rust source path from user options. * Raise errors in constructor, if anything goes wrong (racerd not available or rust source is not available) ycmd will stop trying. * Start server on OnFileReadyToParse. racerd will only start if user is editing a rust file. * Use utils.GetUnusedLocalhostPort() to get a random port and start racerd with this port. * Use localhost and port from GetUnusedLocalhostPort for 'target' variable instead of parsing racerd message. * Check phandle to determine ServerIsRunning. * Fix blank lines before methods. * Fix copyright holders and typo
|
Hi @onur, Thanks for showing interest in this work! I'm happy to see I'm not the only one who wishes to see YCM's Rust support improve. :D I'm currently working on this extensively on my local machine so these changes can't easily be reconciled with what I have and with what I intend to add; it's all still in a state of flux. With that in mind, PR's against the current work aren't the best fit so I'll close this. When I have a more stable version that's also merged into ycmd, I encourage you to send PRs then! :) |
|
Well I don't know what your intentions are but this PR was fixing most of the issues mentioned in ycm-core#268 but whatever. |
Hi. I just made some improvements to rustc_completer (But, I just saw @vheon's review in your PR ycm-core#268). I hope you can keep your focus on racerd (hopefully implement a cache) while I am working on this. And thanks for your amazing work, I can't wait for ycmd rust completion.
Changes are listed below:
binary path and rust source path from user options.
available or rust source is not available) ycmd will stop trying.
is editing a rust file.
racerd with this port.
variable instead of parsing racerd message.