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