> I think that the code is already an improvement.

Nice!

> May you convert it into a proper patch against the latest main?

Of course! I'll try to do it this weekend.

El sáb, 22 feb 2025 a las 16:52, Ihor Radchenko (<yanta...@posteo.net>)
escribió:

> Héctor Galbis Sanchis <hectometrocuadr...@gmail.com> writes:
>
> >>  #+RESULTS:
> >>  [[file:"plot.png"]]
> >>
> >>So, it is no so simple.
> >
> > I see. I've been looking at ob-emacs-lisp.el since elisp and common lisp
> > are very similar. So, checking if `scalar` or `verbatim` are used should
> be
> > sufficient. If some is being used then returns the result. Otherwise,
> strip
> > out the quotes.
>
> Makes sense.
>
> >>Why list? Maybe just return current-result as is?
> >
> > As the function is recursive, if it returns a non-list object It will be
> > creating a non-proper list.
>
> Right.
>
> > However, this version could be more clear:
>
> +1
>
> > I've been thinking about how scalar and verbatim should work and I end up
> > with the conclusion that we cannot use a function that change the
> separator
> > when using Sly. The reason is that using `read-from-string` can fail in a
> > lot of different scenarios. Every literal object whose representation in
> > Common Lisp differs from the representation in Emacs Lisp will fail. This
> > includes vectors, characters, or even symbols.
> > ...
>
> Indeed. `read-from-string' is simply a quick and dirty way to make
> things working without having to implement a dedicated CL reader.
>
> > Besides, regarding the separator ', ', the function that writes the
> results
> > is the next one:
> > ...
> > ```
> > (defun echo-for-emacs (values &optional (fn #'slynk-pprint))
> > ...
> >              (if (some #'(lambda (s) (find #\Newline s))
> >                        strings)
> >                  (format nil "~{~a~^~%~}" strings)
> >                  (format nil "~{~a~^, ~}" strings)))))))
> > ```
> >
> > Check out the last 4 lines. It is weird that they are printing newlines
> > only when a newline is in one of the values. It seems like they put the
> > `format` functions in the wrong order.
>
> I am not sure where this `echo-for-emacs' is coming from.
>
> > Another thing I don't understand is the use of
> > `org-babel-lisp-vector-to-list`. I mean, clearly it is used to let
> vectors
> > being readable by emacs lisp. But, consider this example:
> >...
> > ```
> > #+begin_src lisp
> >   "This is a vector in Common Lisp: #(1 2 3)"
> > #+end_src
> >
> > #+RESULTS:
> > : This is a vector in Common Lisp: (1 2 3)
> > ```
> >
> > This feels wrong to me. But maybe there is a case where this is
> necessary.
> > Clearly, this is up to you, but I'm curious about the reason of using
> > `org-babel-lisp-vector-to-list`.
>
> Well.
>
> (defun org-babel-lisp-vector-to-list (results)
>   "Convert #(...) values in RESULTS string into a (...) list."
>   ;; TODO: better would be to replace #(...) with [...]
>   (replace-regexp-in-string "#(" "(" results))
>
> and the relevant commit says
>
> ob-lisp: turning vector results into lists for easy elisp reading
>
> * lisp/ob-lisp.el (org-babel-execute:lisp): Turn vectors into lists
>   before reading by elisp
>   (org-bable-lisp-vector-to-list): Stub of a vector->list function,
>   should be replaced with a cl-vector->el-vector function.
>
> In other words, this whole thing is a workaround to make
> `read-from-string' work a bit better yet without implementing a
> dedicated CL reader.
>
> > Lastly, this is the code I have right now:
>
> Thanks!
> I think that the code is already an improvement.
> May you convert it into a proper patch against the latest main?
> See https://orgmode.org/worg/org-contribute.html#first-patch
>
> > ```
> > (defun org-babel-read-lisp-values (result)
> >   "Parse RESULT as a sequence of values.
> >
> > RESULT is a string containing one or more Lisp values, optionally
> > separated by commas. The function reads each expression sequentially
> > and returns the parsed values as a list."
> >   (if (string-empty-p result)
> >       nil
> >     (let* ((r (read-from-string result))
> >            (current-result (car r))
> >            (next-char (cdr r))
> >            (rest-result (if (< next-char (length result))
> >                             (if (char-equal (aref result next-char) ?\,)
> >                                 (substring result (+ next-char 2))
> >                               (substring result (1+ next-char)))
> >                           (substring result next-char))))
> >       (cons current-result (org-babel-read-lisp-values rest-result)))))
> >
> > (defun org-babel-execute:lisp (body params)
> >   "Execute a block of Common Lisp code with Babel.
> > BODY is the contents of the block, as a string.  PARAMS is
> > a property list containing the parameters of the block."
> >   (let (eval-and-grab-output)
> >     (pcase org-babel-lisp-eval-fn
> >       (`slime-eval (org-require-package 'slime "SLIME")
> >                    (setq eval-and-grab-output
> 'swank:eval-and-grab-output))
> >       (`sly-eval (org-require-package 'sly "SLY")
> >                  (setq eval-and-grab-output
> 'slynk:eval-and-grab-output)))
> >     (org-babel-reassemble-table
> >      (let* ((result-params (cdr (assq :result-params params)))
> >             (result
> >              (funcall (if (member "output" result-params)
> >                           #'car #'cadr)
> >                       (with-temp-buffer
> >                         (insert (org-babel-expand-body:lisp body params))
> >                         (funcall org-babel-lisp-eval-fn
> >                                  `(,eval-and-grab-output
> >                                    ,(let ((dir (if (assq :dir params)
> >                                                    (cdr (assq :dir
> params))
> >                                                  default-directory)))
> >                                       (format
> >                                        (if dir (format
> > org-babel-lisp-dir-fmt dir)
> >                                          "(progn %s\n)")
> >                                        (buffer-substring-no-properties
> >                                         (point-min) (point-max)))))
> >                                  (cdr (assq :package params)))))))
> >        (org-babel-result-cond result-params
> >          (if (or (member "scalar" result-params)
> >                  (member "verbatim" result-params))
> >              result
> >            (org-strip-quotes result))
> >          (condition-case nil
> >              (let ((list-values (org-babel-read-lisp-values
> > (org-babel-lisp-vector-to-list result))))
> >                (if (length= list-values 1)
> >                    (car list-values)
> >                  list-values))
> >            (error result))))
> >      (org-babel-pick-name (cdr (assq :colname-names params))
> >                           (cdr (assq :colnames params)))
> >      (org-babel-pick-name (cdr (assq :rowname-names params))
> >                           (cdr (assq :rownames params))))))
> > ```
> >
> > El mar, 18 feb 2025 a las 19:00, Ihor Radchenko (<yanta...@posteo.net>)
> > escribió:
> >
> >> Héctor Galbis Sanchis <hectometrocuadr...@gmail.com> writes:
> >>
> >> > I’m new to Org mode and I didn’t know these parameters even exist.
> >> >
> >> > So, I have studied a bit how it should work, and I think I’ve came up
> >> with
> >> > a good solution.
> >> > Besides, I think I’ve fixed some inconsistencies.
> >>
> >> Thanks for working on this!
> >>
> >> > First of all, look at these examples using emacs lisp:
> >> > ...
> >> > #+begin_src emacs-lisp :results output
> >> > (prin1 "Hello")
> >> > #+end_src
> >> >
> >> > #+RESULTS:
> >> > : "Hello"
> >> > ...
> >> > #+begin_src lisp :results output
> >> >   (prin1 "Hello")
> >> > #+end_src
> >> >
> >> > #+RESULTS:
> >> > : Hello
> >> >
> >> > This is due to org-babel-execute:lisp calling org-strip-quotes. I
> removed
> >> > it. Also, this fixes the verbatim option:
> >>
> >> `org-strip-quotes' is there for a reason. Try
> >>
> >>   #+BEGIN_SRC lisp :results file
> >>   "plot.png"
> >>   #+END_SRC
> >>
> >>   #+RESULTS:
> >>   [[file:plot.png]]
> >>
> >> With your patch, you will get
> >>
> >>   #+BEGIN_SRC lisp :results file
> >>   "plot.png"
> >>   #+END_SRC
> >>
> >>   #+RESULTS:
> >>   [[file:"plot.png"]]
> >>
> >> So, it is no so simple.
> >>
> >> > Now, let’s talk about the design of returning multiple values and how
> >> they
> >> > should work. I decided that returning multiple values should behave as
> >> > returning a list. I think it makes sense, since multiple values can be
> >> seen
> >> > as a list of values.
> >>
> >> Makes sense.
> >>
> >> > Lastly, here is the code fixing the inconsistencies and adding
> support of
> >> > returning multiple values:
> >> > ...
> >> > (defun org-babel-change-sly-separator (result)
> >> >   (let ((new-result "")
> >> >         (next-char 0))
> >> >     (while (< next-char (length result))
> >> >       (let* ((r (read-from-string result next-char))
> >> >              (current-result (substring result next-char (cdr r))))
> >> >         (setq new-result (if (zerop next-char)
> >> >                              current-result
> >> >                            (concat new-result "\n" current-result))
> >> >               next-char (+ (cdr r) 2))))
> >> >     new-result))
> >>
> >> This is not ideal, actually. Consider really large output. Then, calling
> >> `read-from-string' will take time and your code will be doing it twice.
> >>
> >> > (defun org-babel-read-values (result)
> >> >   "Read all results and tunrs ', ' into newlines."
> >>
> >> This docstring seems to be out of scope.
> >>
> >> >   (let* ((r (read-from-string result))
> >> >          (current-result (car r))
> >> >          (next-char (cdr r))
> >> >          (rest-result (substring result next-char)))
> >> >     (if (string-empty-p rest-result)
> >> >         (list current-result)
> >>
> >> Why list? Maybe just return current-result as is?
> >>
> >> Also, may you edit ob-lisp.el from the latest main? It has some
> >> differences.
> >>
> >> --
> >> Ihor Radchenko // yantar92,
> >> Org mode maintainer,
> >> Learn more about Org mode at <https://orgmode.org/>.
> >> Support Org development at <https://liberapay.com/org-mode>,
> >> or support my work at <https://liberapay.com/yantar92>
> >>
>
> --
> Ihor Radchenko // yantar92,
> Org mode maintainer,
> Learn more about Org mode at <https://orgmode.org/>.
> Support Org development at <https://liberapay.com/org-mode>,
> or support my work at <https://liberapay.com/yantar92>
>

Reply via email to