ahyatt / emacs-websocket

A websocket implementation in elisp, for emacs.
GNU General Public License v2.0
325 stars 42 forks source link

Support onopen event handler #7

Closed tkf closed 12 years ago

tkf commented 12 years ago

See: http://dev.w3.org/html5/websockets/#the-websocket-interface

Problem is that you will need to parse handshake reply from server. I think implementing handshake parser for current supported version (draft 76) is useless as it is old. Do you think we should wait until RFC 6455 implementation (#1) is finished? For now, I am working around this problem simply sleep after connecting server and call "onopen" function.

ahyatt commented 12 years ago

I think it's fine to ignore this for draft 76 for now. I'm not sure how long it will take me to implement RFC 6455, but I'll add the handshake parsing in there. Eventually, we should switch completely to RFC 6455, and remove support for draft 76.

On Sun, May 6, 2012 at 8:40 PM, Takafumi Arakaki reply@reply.github.com wrote:

See: http://dev.w3.org/html5/websockets/#the-websocket-interface

Problem is that you will need to parse handshake reply from server.  I think implementing handshake parser for current supported version (draft 76) is useless as it is old.  Do you think we should wait until RFC 6455 implementation (#1) is finished? For now, I am working around this problem simply sleep after connecting server and call "onopen" function.


Reply to this email directly or view it on GitHub: https://github.com/ahyatt/emacs-websocket/issues/7

tkf commented 12 years ago

Yeah, I agree with you about completely switching to RFC 6455.

tkf commented 12 years ago

When we switch to RFC 6455, how about changing API also to follow W3C? I think this makes easier to understand API, especially for programmers who use javascript. Something like this:

(defstruct websocket
  ;; -- Internal variables
  (conn (assert nil) :read-only t)
  (inflight-packet nil)
  ;; -- Attributes defined in W3C API
  (url (assert nil) :read-only t)
  (ready-state nil)
  ;; callbacks
  (on-open (assert nil) :read-only t)
  (on-error (assert nil))      ; It should be OK to re-define
  (on-close (assert nil))      ; these attributes after opening
  (on-message (assert nil))    ; socket, no?
  ;; arguments for callbacks
  (on-open-args (assert nil) :read-only t)
  (on-error-args (assert nil))
  (on-close-args (assert nil))
  (on-message-args (assert nil)))

I am not sure having all these on-EVENT-args is a good choice. Other alternatives I can think of:

Also, how about changing prefix to websocket-client from websocket? Maybe we want to have websocket server at some point.

ahyatt commented 12 years ago

This is a good suggestion, thanks! I probably will get to this after the current set of changes I'm doing on the frame protocol. I think I like the lexical-let version the best. We can also make sure that the websocket callbacks all happen in the context of a buffer, and have that be the only "argument". The client can then use buffer-local variables. Let me think about this, but in general your idea sounds good and I'll implement it like the w3c spec in the near future.

On Sat, May 19, 2012 at 2:31 PM, Takafumi Arakaki reply@reply.github.com wrote:

When we switch to RFC 6455, how about changing API also to follow W3C?  I think this makes easier to understand API, especially for programmers who use javascript. Something like this:

(defstruct websocket
 ;; -- Internal variables
 (conn (assert nil) :read-only t)
 (inflight-packet nil)
 ;; -- Attributes defined in W3C API
 (url (assert nil) :read-only t)
 (ready-state nil)
 ;; callbacks
 (on-open (assert nil) :read-only t)
 (on-error (assert nil))      ; It should be OK to re-define
 (on-close (assert nil))      ; these attributes after opening
 (on-message (assert nil))    ; socket, no?
 ;; arguments for callbacks
 (on-open-args (assert nil) :read-only t)
 (on-error-args (assert nil))
 (on-close-args (assert nil))
 (on-message-args (assert nil)))

I am not sure having all these on-EVENT-args is a good choice. Other alternatives I can think of:

  • Make on-EVENT slot hold cons (FUNCTION . ARGS) and callback is called as (apply FUNCTION packet ARGS).
  • Add miscellaneous slot to websocket and callback is called as (funcall FUNCTION packet websocket).
  • Add MAKE-WEBSOCKET argument (default value is 'make-websocket) to websocket-open and callback is called as (funcall FUNCTION packet websocket).  This way, user can extend websocekt struct and put stuff he wants to hold in his extra slot.
  • Forget about arguments.  Use lexical-let.

Also, how about changing prefix to websocket-client from websocket?  Maybe we want to have websocket server at some point.


Reply to this email directly or view it on GitHub: https://github.com/ahyatt/emacs-websocket/issues/7#issuecomment-5803826

ahyatt commented 12 years ago

BTW, about changing websocket to websocket-client - yes, it's a good idea, but the name is long, and given how we'll have to prefix everything by this name, I wonder if we should instead use ws-client or something like that. This is something we should do later, I think.

On Sat, May 19, 2012 at 7:27 PM, Andrew Hyatt ahyatt@gmail.com wrote:

This is a good suggestion, thanks!  I probably will get to this after the current set of changes I'm doing on the frame protocol.  I think I like the lexical-let version the best.  We can also make sure that the websocket callbacks all happen in the context of a buffer, and have that be the only "argument".  The client can then use buffer-local variables.   Let me think about this, but in general your idea sounds good and I'll implement it like the w3c spec in the near future.

On Sat, May 19, 2012 at 2:31 PM, Takafumi Arakaki reply@reply.github.com wrote:

When we switch to RFC 6455, how about changing API also to follow W3C?  I think this makes easier to understand API, especially for programmers who use javascript. Something like this:

(defstruct websocket
 ;; -- Internal variables
 (conn (assert nil) :read-only t)
 (inflight-packet nil)
 ;; -- Attributes defined in W3C API
 (url (assert nil) :read-only t)
 (ready-state nil)
 ;; callbacks
 (on-open (assert nil) :read-only t)
 (on-error (assert nil))      ; It should be OK to re-define
 (on-close (assert nil))      ; these attributes after opening
 (on-message (assert nil))    ; socket, no?
 ;; arguments for callbacks
 (on-open-args (assert nil) :read-only t)
 (on-error-args (assert nil))
 (on-close-args (assert nil))
 (on-message-args (assert nil)))

I am not sure having all these on-EVENT-args is a good choice. Other alternatives I can think of:

  • Make on-EVENT slot hold cons (FUNCTION . ARGS) and callback is called as (apply FUNCTION packet ARGS).
  • Add miscellaneous slot to websocket and callback is called as (funcall FUNCTION packet websocket).
  • Add MAKE-WEBSOCKET argument (default value is 'make-websocket) to websocket-open and callback is called as (funcall FUNCTION packet websocket).  This way, user can extend websocekt struct and put stuff he wants to hold in his extra slot.
  • Forget about arguments.  Use lexical-let.

Also, how about changing prefix to websocket-client from websocket?  Maybe we want to have websocket server at some point.


Reply to this email directly or view it on GitHub: https://github.com/ahyatt/emacs-websocket/issues/7#issuecomment-5803826

tkf commented 12 years ago

I guess you are planning to start versioning after changing protocol, but please make sure do it before changing API! :)

About using buffer as "argument", I am not sure if it is a good idea because you can think of an application which has multiple buffers sharing connection. Making a special buffer for holding connection sounds a little bit redundant.

Emacs lisp functions seem to take callback argument usually. For example, run-at-time, run-with-idle-timer, url-retrieve and make-network-process. I agree that having all these on-EVENT-args slots is not cool. I'd go with "packing by cons" or "miscellaneous slot" idea.

BTW, If you go with "miscellaneous slot" idea, probably the callback must be called via (funcall FUNCTION websocket packet). In this way, we can think like FUNCTION is "overriding" default callback function, like Python method takes self (=websocket) as the first argument.

About websocket-client, yea, I know it's too long. I hope Emacs has much more sophisticated name space management... One thing we can do is to use EIEIO and use same function name for server and client.

ahyatt commented 12 years ago

Sorry, I didn't understand what you mean by "versioning" here? My plan is to finish all the receiving & sending code (should take me another few days), make sure everything actually works in practice, then I'll probably merge it back to the master branch.

I may or may not want to change the API in this branch. One one hand, the API should be distinct from the protocol. But, actually, I'm unsure this is the case. Right now I've already started passing the websocket frame itself, instead of just the frame payload. The issue is that I think I need to pass in whether the frame is a continuation frame or not. The W3C API doesn't seem to deal with this, but according to the spec it might be important, depending on what extensions are used.

On Sat, May 19, 2012 at 8:26 PM, Takafumi Arakaki reply@reply.github.com wrote:

I guess you are planning to start versioning after changing protocol, but please make sure do it before changing API! :)

About using buffer as "argument", I am not sure if it is a good idea because you can think of an application which has multiple buffers sharing connection.  Making a special buffer for holding connection sounds a little bit redundant.

Emacs lisp functions seem to take callback argument usually.  For example, run-at-time, run-with-idle-timer, url-retrieve and make-network-process.  I agree that having all these on-EVENT-args slots is not cool.  I'd go with "packing by cons" or "miscellaneous slot" idea.

BTW, If you go with "miscellaneous slot" idea, probably the callback must be called via (funcall FUNCTION websocket packet).  In this way, we can think like FUNCTION is "overriding" default callback function, like Python method takes self (=websocket) as the first argument.

About websocket-client, yea, I know it's too long.  I hope Emacs has much more sophisticated name space management...  One thing we can do is to use EIEIO and use same function name for server and client.


Reply to this email directly or view it on GitHub: https://github.com/ahyatt/emacs-websocket/issues/7#issuecomment-5805785

tkf commented 12 years ago

Sorry, I didn't understand what you mean by "versioning" here?

Sorry, I was not clear. I mean to release websocket.el and attach a version number to that release; add websocket-version variable, add tag in the git repository and maybe put in marmalade.

I may or may not want to change the API in this branch.

Yes, I think it's better to do it after merge this in master, probably in a separate branch again.

I am OK with extra/different arguments. Maybe we can just use the function/attribute naming of the W3C API.

ahyatt commented 12 years ago

I'm closing this as fixed, but I made the W3C compliance another issue, which is next on my list of things to do.