Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add compile file command #79

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add compile file command #79

wants to merge 1 commit into from

Conversation

neil-lindquist
Copy link
Contributor

This adds the interface for compiling a file. This fixes #37 and makes #41 obsolete.
This pull request depends on sjlevine/swank-client-js#6.

@vindarel
Copy link

Fantastic work. At this rhythm Atom-Slime will soon fix the n°1 complain heard for Lisp, "there is no decent editor besides Emacs" (which is more and more untrue, given also Vim, Lem, the recently discovered Dandelion plugin for Eclipse, the ipython notebook, etc).

@sjlevine
Copy link
Owner

This is also a very cool feature, thank you for the contribution here and to swank-client-js!

This seems to work great for files encoded in ASCII! However, I've found a problem when using non-ASCII files. As there are many users in countries around the world using this package with non-English characters, we'd need to get to the bottom of this before merging.

Here is what I get when I try to compile a file that is encoded in UTF-8:

; compiling file "<filename here>" (written 07 DEC 2018 10:20:29 PM):
; 
; caught ERROR:
;   READ error during COMPILE-FILE:
;   
;     :ASCII stream decoding error on
;     #<SB-INT:FORM-TRACKING-STREAM
;       for "file <filename here>"
;       {1008775F03}>:
;   
;       the octet sequence #(226) cannot be decoded.
;   
;     (in form starting at line: 60, column: 0, position: 1977)
; 
; compilation unit aborted
;   caught 1 fatal ERROR condition
;   caught 1 ERROR condition
; compilation aborted after 0:00:00.021

CL-USER> 

It's not immediately clear to me why swank isn't handling the encoding correctly. It seems to work fine in Emacs though, so I checked what commands it sends to swank (I'm guessing you've already seen the wiki page on using my custom swank server that's instrumented to print out everything -- great for understanding the protocol). Here's what commands swank get with Emacs:

Read: (:EMACS-REX
 (SWANK:COMPILE-FILE-FOR-EMACS
  "<filename>.lisp"
  T)
 "#:cl-user" T 14)
Write: (:WRITE-STRING
 "; compiling file \"<filename>\" (written 11 DEC 2018 07:30:31 PM):
")
Write: (:WRITE-STRING "
; <filename>.fasl written
")
Write: (:WRITE-STRING "; compilation finished in 0:00:00.114
")
Write: (:RETURN
 (:OK
  (:COMPILATION-RESULT NIL T 0.115 T
   "<filename>.fasl"))
 14)
Read: (:EMACS-REX
 (SWANK:LOAD-FILE
  "<filename>.fasl")
 "#:pike" T 15)
Write: (:RETURN (:OK "T") 15)

That seems to pretty similar to what you're doing in swank-client-js (except maybe for the SWANK:LOAD-FILE part, but that's not what seems to be giving the error). So again, not immediately clear to me why it isn't working right now.

Unfortunately I don't have the cycles to debug now (writing my thesis), but if you happen to catch the bug, I'm all ears! No worries if you need a break though, it seems you've spent quite a bit of time lately on this package. :-)

Thanks again for all your hard work. I haven't looked at if #76 places nicely with your other changes, but it has not been forgotten.

@neil-lindquist
Copy link
Contributor Author

I think the issue is SBCL's default external format. If sb-impl::*default-external-format* is :utf-8, then it works correctly. Otherwise, it gives that error (with :ascii changed to the encoding in use).
I feel like adjusting the external format isn't something that this package should do since users may need to use legacy encodings and SBCL is already using the user's default).

I'm not sure why Emacs works for you, but Atom-Slime doesn't. Is sbcl launched differently or something?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add "compile this file"
3 participants