joaotavora / sly

Sylvester the Cat's Common Lisp IDE
1.23k stars 139 forks source link

Implement sly-media.el #638

Closed jcguu95 closed 2 months ago

jcguu95 commented 2 months ago

Thank svetlyak40wt for bringing up the issue! With a few tweaks, slime-media.el is now ported to sly in this contrib!

With this contrib, we can display image on disk in sly buffers. We can also call that from a Lisp that connects to the sly buffer following the steps below.

  1. Define function in the Lisp that connects to the current sly buffer.
(defun display-image-on-disk (filename)
  \"Displays an image in the current sly buffer.\"
  (slynk:eval-in-emacs
   `(sly-media-insert-image
     (create-image ,filename) ,filename)))
  1. Evaluate in emacs: (setq sly-enable-evaluate-in-emacs t)
  2. Evaluate in the sly buffer: (display-image-on-disk "/tmp/example.jpg")

It is totally usable already, but it can be better. Please take a look at the function sly-dispatch-media-event (currently commented out). In particular, I wonder how slynk triggers an automatic event, which under the macrosly-dcase has car :write-image. If someone could let me know, I'm happy to integrate it into this contrib.

Any comment is welcome!

svetlyak40wt commented 2 months ago

Cool! This works for me!

However it does nothing if image file is missing. On this screenshot cat2.jpg is missing:

Screenshot 2024-04-23 at 12 11 36

Probably some error should be raise if file is missing?

joaotavora commented 2 months ago

IMO this isn't really what a contrib such as this should do. It's not sending images through the REPL, it's sending filenames, so it's that much more useful than just calling SLYNK:EVAL-IN-EMACS with some half-trivial Elisp function sitting in your config. And it's fairly useless if the SLY client and the SLYNK server are in different hosts, right? A much nicer contrib with a beefier CL Slynk would encode and send actual (compressed?) binary data through the wire along with a notice of the content type being exchanged. Much like HTTP really.

jcguu95 commented 2 months ago

Thanks for the feedback!

@joaotavora Not really. You can send actual image data (as cons, for example) using this contrib (in the very first commit 9c802fe) and have the image inserted correctly. For example,

;;; Common Lisp
(slynk:eval-in-emacs
  ;; Emacs Lisp Data
 `(sly-media-insert-image 
    ;; This is just a cons cell. See footnote [1].
   '(image
     :type svg
     :data "<svg width=\"300\" height=\"200\" version=\"1.1\" xmlns=\"http://www.w3.org/2000/svg\" xmlns:xlink=\"http://www.w3.org/1999/xlink\"> <rect width=\"100%\" height=\"100%\" x=\"0\" y=\"0\" fill=\"red\"></rect> <circle cx=\"150\" cy=\"100\" r=\"80\" fill=\"green\"></circle> <text y=\"125\" x=\"150\" text-anchor=\"middle\" font-size=\"60\" fill=\"white\"> Hello!</text></svg>"
     :scale 1.1068702290076335
     :transform-smoothing t
     :background "#1d2021")
   "Good afternoon!"))

(I need to document this usage in the code before it's merged.)

#'sly-media-insert-image uses elisp's #'insert-image to insert images. As that is well documented in the elisp manual, I believe this is the correct way to insert images in emacs buffers. The problem is that there are many image specifications that elisp supports. For instance, in the original post, (insert-image (create-image "/path/to/local/image.png"). I have yet found out a good way to describe all of them. Perhaps, the burden should be put on the users of this contrib? I can provide as many examples in the source code. Please let me know what you think. Ultimately, the concerning question: How much should sly-media.el do?

(By the way, thanks so much for making sly. It has changed our lives in a very good way!)

@svetlyak40wt Thanks for trying out! Yes, I will add condition handling before it gets merged. For now, I'd like to hear from you and @joaotavora about the aforementioned concern.

Footnote

;;; Emacs Lisp
(svg-image
 '(svg ((width . 300) (height . 200) (version . 1.1)
        (xmlns . http://www.w3.org/2000/svg)
        (xmlns:xlink . http://www.w3.org/1999/xlink))
   (rect ((width . 100%) (height . 100%)
          (x . 0) (y . 0) (fill . red)))
   (circle ((cx . 150) (cy . 100) (r . 80) (fill . green)))
   (text ((y . 125) (x . 150)
          (text-anchor . middle)
          (font-size . 60) (fill . white))
         "Hello!")))
jcguu95 commented 2 months ago

@joaotavora Besides svg data, elisp #'create-image works for many other types of image data (e.g. png, jpg..). When it sees a file path, elisp calls a converter to convert the file into an image datum it understands before displaying. In this case, it is using ffmpeg as the converter.

Proposal: I would imagine a less intrusive way of design for displaying remote image data is by first saving the image as a temporary file, and let elisp #'create-image handles the rest. What do you think?

joaotavora commented 2 months ago

What do you think?

I think you're just creating a very thin shell to write Elisp code inside Common Lisp, and that isn't worth a contrib. IOW I don't see why your display-image-on-disk function, which only allows for filenames as far as I can tell, and which isn't even in the contrib, needs to be implemented in terms of sly-media-insert-image, which is just trivial Elisp.

Finally, what I would like is a way for the result of certain expressions to act like a presentation. Similar to SLIME presentations, but not so intrusive. A CL function that returns a CL object of a certain special type would get special treatment in the REPL. A good sly-media.el contrib would allow:

This is not a trivial effort, but it is possible (and likely needs some changes to SLYs base machinery), but it is the effort that would be worth merging.

jcguu95 commented 2 months ago

@joaotavora Thank you for writing up your thoughts. I did not know slime presentations, but it is something that is very good to have! As you said, the effort to achieve is nontrivial, so I can't do that now on my own.. (sorry @svetlyak40wt ..). Anyone interested in implementing this and needs help, feel free to let me know!

I will delete my branch soon. Anyone wants to use this script can find it below.

;;; -*-lexical-binding:t-*-
(require 'sly)
(require 'cl-lib)

(define-sly-contrib sly-media
  "Display images in SLY buffers"
  (:authors "Jin-Cheng Guu <jcguu95@gmail.com>")
  (:license "GPL")
  ;; (:on-load
  ;;  (add-hook 'sly-event-hooks 'sly-dispatch-media-event))
  )

(defun sly-media-insert-image (image string)
  "Insert image into the current sly buffer. Example Usage:
(1) Define function in the Lisp that connects to the current sly buffer.
    ; (defun display-image-on-disk (filename)
    ;   \"Displays an image in the current sly buffer.\"
    ;   (slynk:eval-in-emacs
    ;    `(sly-media-insert-image
    ;      (create-image ,filename) ,filename)))
(2) Evaluate in emacs: (setq sly-enable-evaluate-in-emacs t)
(3) Evaluate in the sly buffer: (display-image-on-disk \"/tmp/example.jpg\")
"
  (with-current-buffer
      (get-buffer (sly-buffer-name :mrepl :connection (sly-current-connection)))
    (let ((marker (sly-mrepl--mark)))
      (goto-char marker)
      (sly-propertize-region `(face slime-mrepl-output-face
                               rear-nonsticky (face))
        (insert-image image string))
      (set-marker marker (point)))
    nil))

;; (defun sly-dispatch-media-event (event)
;;   (sly-dcase event
;;     ((:write-image image string)
;;      (cl-labels
;;          ((media-decode-image (image)
;;             (mapcar (lambda (image)
;;                       (if (plist-get image :data)
;;                           (plist-put image :data (base64-decode-string
;;                                                   (plist-get image :data)))
;;                         image))
;;                     image)))
;;        (let ((img (or (find-image (media-decode-image image))
;;                       (create-image image))))
;;          (sly-media-insert-image img string)))
;;      t)
;;     (t nil)))

(provide 'sly-media)