Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

swank protocol header use chars length rather than bytes && fix swank server address #37

Closed
wants to merge 5 commits into
from

Conversation

Projects
None yet
4 participants

hi, recently i'm using swank-js for remote debugging, and it's quite handy.

During using it, i found two problems:

  • it doesn't support utf-8 properly.
  • swank-js-inject.js use the injected page address as swank server address

For ther first one, If my understanding is right, Swank protocol header use chars length rather than bytes.
For the second, i make swank-js-inject use it's script tag address.

This currently works well for me.

This behavior is consistent with older versions of SLIME. If you look at the commit history of swank-js, you'll notice
that it was counting characters too some time ago. But this isn't compatible with recent SLIMEs

I'm using the latest slime version from git://sbcl.boinkor.net/slime.git, and from the slime.el:

(defun slime-net-send (sexp proc)
  "Send a SEXP to Lisp over the socket PROC.
This is the lowest level of communication. The sexp will be READ and
EVAL'd by Lisp."
  (let* ((payload (encode-coding-string
                   (concat (slime-prin1-to-string sexp) "\n")
                   'utf-8-unix))
         (string (concat (slime-net-encode-length (length payload))
                         payload)))
    (slime-log-event sexp)
    (process-send-string proc string)))

Here it use length function to get the char lengths of payload.

Owner

jonnay commented Jul 5, 2012

There are a lot of other great changes to this pull request that should get merged in, but I haven't had a chance to get to it. In particular slime-js-pst looks pretty groovy.

Owner

jonnay commented Jul 9, 2012

I've checked it out, and xianaitong is correct, slime.el is indeed counting characters.

I18n is pretty important, so I'll try to get at this pull request asap, and once that is done, I can remove slime.el and close off issue #34

osener commented Oct 5, 2012

There are indeed some useful changes in this pull request. Any idea when the changes that are not related to xiaonaitong/swank-js@fcb07ba might make it into master?

Owner

jonnay commented Oct 13, 2012

Okay, I have merged this pull request, but I want to keep this issue open until I am sure it works. I did some cursory tests, but it seemed to work!

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