Skip to content
This repository was archived by the owner on Sep 21, 2019. It is now read-only.

Conversation

@rorygraves
Copy link
Contributor

No description provided.

import context.{ dispatcher, system }

import FileUtils._
import org.ensime.util.FileUtils._
Copy link
Contributor

Choose a reason for hiding this comment

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

this looks a bit gnarly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was their already - I made it fully qualified rather than having FileUtils imported above and then _ imported here.

@fommil
Copy link
Contributor

fommil commented Jul 9, 2016

LGTM

@fommil
Copy link
Contributor

fommil commented Jul 9, 2016

emacs is getting caught up on this http://fommil.com/ensime/ensime-server/765

should we disable the emacs tests in the server and fix it client side?

@fommil
Copy link
Contributor

fommil commented Jul 9, 2016

it would be good if the project sent a persisted connection-info message during startup. That simplifies the client code because it means you don't need to wait for the connection and then send a message and wait for a response... you can just fire off connection handling logic and wait async until the connection info returns.

@rorygraves
Copy link
Contributor Author

It can't - its a response to a ConnectionInfoRequest
The server will respond correctly to that message - any will send the aysnc messages as an when they aer available (possibly immediately on connection). These feels like hack logic that should not exist on the server.

@rorygraves
Copy link
Contributor Author

You should just be able to connect and go full async - assume the server is up and wait for the ready messages to tell you when its available for work.
You probably don't need to send ConnectionInfoReq at all

@rorygraves
Copy link
Contributor Author

I suspect you don't check the result - so its just a ping-pong.

@rorygraves
Copy link
Contributor Author

Ok, the emacs expectations about startup behaviour are subtle and I'm going to have to introduce a new hack until such times as we fix the protocol (it assumes that the first thing it will ever see is a response to the a connectionInfoReq and not any other async message).

@fommil fommil mentioned this pull request Jul 9, 2016
@aemoncannon
Copy link
Contributor

Some history: we were adjusting our requests based on the protocol version, but there was no deterministic ordering of startup messages, so we'd end up firing early RPCs not compatible with the current protocol. That's why we do the blocking handshake before anything else. Any plan to get rid of the handshake should probably include a revised plan for handling protocol evolution.

References:
#1142 (comment)
https://github.com/ensime/ensime-emacs/pull/282/files
ensime/ensime-emacs#284 (comment)

@aemoncannon
Copy link
Contributor

The elisp tests are designed to ensure we get the protocol version before compiler ready is sent (because compiler ready kicks off a bunch of other rpcs).

@rorygraves
Copy link
Contributor Author

Yeah, I'm going to do this in two passes:

  1. Tweak the above to ensure that the ConnectionInfo is handled first (i.e. implement the current behaviour, but not in Project)
  2. Move to a welcome message (i.e. first message on channel will be a version info - you don't need to request it - and kill off that syncronous message.

@fommil
Copy link
Contributor

fommil commented Oct 30, 2016

dead as a dodo, could form the basis of a revitalised attempt though, coming soon.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants