org-babel guile source block bug in handling multiple values

org-babel guile source block bug in handling multiple values

[Zelphir Kaltstahl]

[-- Attachment #1: Type: text/plain, Size: 4465 bytes --]

Hello Org users!

I think I found a bug in how org-babel works with at least GNU Guile source 
blocks. I could not reproduce this for example with a racket source block, 
because somehow there is no ob-racket and I think I would need that. I could try 
other Schemes like maybe Chicken, if there is ob-chicken or so available. The 
issue happens on 2 separate machines.

I am not sure on which repository I should open an issue for this specific bug, 
or if perhaps this mailing list is exactly the right place. Feel free to point 
me elsewhere : )

What follows are my notes trying to narrow it down, as an org document.

~~~~START~~~~
* General

There seems to be an issue with returning multiple values, which result from =:var= variables / whose computation involves =:var= variables. Handling multiple trivial values like ~1~ or ~2~ seems to work fine.

* Primitive multiple values work

#+begin_src scheme :eval query-export :results output replace drawer
(import (except (rnrs base) error vector-map)
         (only (guile)
               lambda*
               λ)
         ;; let-values
         (srfi srfi-11))

(let-values ([(a b) (values 1 2)])
   (simple-format #t "~a ~a\n" a b))
#+end_src

#+RESULTS:
:results:
1 2
:end:

* Multiple values involving =:var= variables

#+begin_src scheme :eval query-export :results output replace drawer :var x=1 :var y=2
(import (except (rnrs base) error vector-map)
         (only (guile)
               lambda*
               λ)
         ;; let-values
         (srfi srfi-11))

(let-values ([(a b) (values x y)])
   (simple-format #t "~a ~a\n" a b))
#+end_src

#+RESULTS:
:results:
ice-9/boot-9.scm:1685:16: In procedure raise-exception:
Unbound variable: a

Entering a new prompt.  Type `,bt' for a backtrace or `,q' to continue.
scheme@(guile-user) [1]>
:end:

** Is it about =:var= variables in values?

#+begin_src scheme :eval query-export :results output replace drawer :var x=1 :var y=2
(import (except (rnrs base) error vector-map)
         (only (guile)
               lambda*
               λ)
         ;; let-values
         (srfi srfi-11))

(define plus-x
   (λ (num)
     (+ num x)))

(define plus-y
   (λ (num)
     (+ num y)))

(let-values ([(a b) (values (plus-x 1) (plus-y 2))])
   (simple-format #t "~a ~a\n" a b))
#+end_src

#+RESULTS:
:results:
ice-9/boot-9.scm:1685:16: In procedure raise-exception:
Unbound variable: a

Entering a new prompt.  Type `,bt' for a backtrace or `,q' to continue.
scheme@(guile-user) [1]>
:end:

** Defining and using defined variables

What about defining and using the defined variables instead of the =:var= variables directly?

#+begin_src scheme :eval query-export :results output replace drawer :var x=1 :var y=2
(import (except (rnrs base) error vector-map)
         (only (guile)
               lambda*
               λ)
         ;; let-values
         (srfi srfi-11))

(define inner-x x)
(define inner-y y)

(define plus-x
   (λ (num)
     (+ num inner-x)))

(define plus-y
   (λ (num)
     (+ num inner-y)))

(let-values ([(a b) (values (plus-x 1) (plus-y 2))])
   (simple-format #t "~a ~a\n" a b))
#+end_src

#+RESULTS:
:results:
ice-9/boot-9.scm:1685:16: In procedure raise-exception:
Unbound variable: a

Entering a new prompt.  Type `,bt' for a backtrace or `,q' to continue.
scheme@(guile-user) [1]>
:end:

It seems not to work whenever the ~(values ...)~ is not only trivial values, but variables or function calls.

* With named :session

Works after running it once with error:

#+begin_src scheme :session mysession :eval query-export :results output replace drawer :var x=1 :var y=2
(import (except (rnrs base) error vector-map)
         (only (guile)
               lambda*
               λ)
         ;; let-values
         (srfi srfi-11))

(let-values ([(a b) (values x y)])
   (simple-format #t "~a ~a\n" a b))
#+end_src

#+RESULTS:
:results:
1 2
:end:

Seems like it gets stored in the session and then can be used in the second run of the source block.
~~~~~END~~~~~

Versions of things:

+ guile @ =3.0.9=
+ org-mode @ =9.6.1=
+ org-babel @ (integrated into org-mode)
+ geiser-guile @ =0.28.1=

Workarounds:

Of course, I could return lists instead of multiple values and then pattern 
match on them or use separate functions for getting the values separately. 
Downside: Might be more computation and requires changing code just, because for 
using it in org source blocks.

-- 
repositories:https://notabug.org/ZelphirKaltstahl

[-- Attachment #2: Type: text/html, Size: 4888 bytes --]

Re: org-babel guile source block bug in handling multiple values

[Ihor Radchenko]

Zelphir Kaltstahl <zelphirkaltstahl@posteo.de> writes:

> * Multiple values involving =:var= variables
>
> #+begin_src scheme :eval query-export :results output replace drawer :var x=1 :var y=2
> (import (except (rnrs base) error vector-map)
>          (only (guile)
>                lambda*
>                λ)
>          ;; let-values
>          (srfi srfi-11))
>
> (let-values ([(a b) (values x y)])
>    (simple-format #t "~a ~a\n" a b))
> #+end_src
>
> #+RESULTS:
> :results:
> ice-9/boot-9.scm:1685:16: In procedure raise-exception:
> Unbound variable: a
>
> Entering a new prompt.  Type `,bt' for a backtrace or `,q' to continue.
> scheme@(guile-user) [1]>
> :end:

I am not familiar with scheme, but AFAIK all ob-scheme does is wrapping
the code block into (let (...) ,@body) See `org-babel-expand-body:scheme'.

May the problem be with your let-values form? 

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
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>

Re: org-babel guile source block bug in handling multiple values

[Zelphir Kaltstahl]

On 3/7/23 15:36, Ihor Radchenko wrote:
> Zelphir Kaltstahl <zelphirkaltstahl@posteo.de> writes:
>
>> * Multiple values involving =:var= variables
>>
>> #+begin_src scheme :eval query-export :results output replace drawer :var x=1 :var y=2
>> (import (except (rnrs base) error vector-map)
>>           (only (guile)
>>                 lambda*
>>                 λ)
>>           ;; let-values
>>           (srfi srfi-11))
>>
>> (let-values ([(a b) (values x y)])
>>     (simple-format #t "~a ~a\n" a b))
>> #+end_src
>>
>> #+RESULTS:
>> :results:
>> ice-9/boot-9.scm:1685:16: In procedure raise-exception:
>> Unbound variable: a
>>
>> Entering a new prompt.  Type `,bt' for a backtrace or `,q' to continue.
>> scheme@(guile-user) [1]>
>> :end:
> I am not familiar with scheme, but AFAIK all ob-scheme does is wrapping
> the code block into (let (...) ,@body) See `org-babel-expand-body:scheme'.
>
> May the problem be with your let-values form?

The problem seems to be associated with returning multiple values, which 
`let-values` is designed to capture. The expression `(values ...)` returns 2 
values and the `(let ([(a b) <multi-value-expression>]) <body>)` will capture 
those 2 values, binding them to `a` and `b`. The `(let-values ...)` then creates 
a value (or in other scenarios any number of values) in its `<body>` part as a 
result of the whole `let-values` expression. In my case the body performs a 
side-effect of printing something out, which is required to get something as an 
output of the source block.

If org merely wraps in a `let`, it should not notice any of the multiple values 
business, because that is something done internally in `let-values`.

I can run the code in geiser manually without error, but when I run it as a 
source block, I get the error as seen in the example org document of my original 
e-mail.

-- 
repositories: https://notabug.org/ZelphirKaltstahl

Re: org-babel guile source block bug in handling multiple values

[Max Nikulin]

On 07/03/2023 18:27, Zelphir Kaltstahl wrote:
> because somehow there is no ob-racket and I think I would need that.

See the following thread for a couple of links. Disclaimer: I have never 
used racket, so I do not know real state of affairs.

https://list.orgmode.org/CA+pajWLyuzVKA3GFSHgXzeqhOfwiVG-FDhe4BPf7Z6fQwVasRg@mail.gmail.com/T/#u
Updates to ob-racket mode, code review request
2020-07-09 15:03 George Mauer

Re: org-babel guile source block bug in handling multiple values

[Bruno Barbier]

Zelphir Kaltstahl <zelphirkaltstahl@posteo.de> writes:


> If org merely wraps in a `let`, it should not notice any of the multiple values 
> business, because that is something done internally in `let-values`.
>

The "let", to define the org variables, ends up putting the "import"
inside the scheme expression, like this:

      ;; -*- geiser-scheme-implementation: guile -*-
      (let ((x '1)
            (y '2))
      (import (except (rnrs base) error vector-map)
               (only (guile)
                     lambda*
                     λ)
               ;; let-values
               (srfi srfi-11))

      (let-values ([(a b) (values x y)])
         (simple-format #t "~a ~a\n" a b))
      )


which raises an error when evaluated the first time (the second time,
the "import" has already imported the new "let-values" feature
(srfi-11), so, the evaluation works). You can test this manually in a
guile session.

I guess you'll have to use sessions, and do the "import" in a separate
block first.

Bruno

Re: org-babel guile source block bug in handling multiple values

[Rudolf Adamkovič]

Zelphir Kaltstahl <zelphirkaltstahl@posteo.de> writes:

> There seems to be an issue with returning multiple values,

By the way, (Common) Lisp also has problems with multiple values in Org, as I
have recently learned in a discussion with the author of SLY (the modern
alternative to SLIME), João Távora.  He says:

> But this is wrong anyway, because in good Lisp REPL tradition, it should
> return the string braced with quotes, so it is READ able, and this doesn't
> work for multiple values anyway.
> 
> #+begin_src lisp
> (values 1 2 3)
> #+end_src
> 
> #+RESULTS:
> : 1

See (at the bottom of):

https://github.com/joaotavora/sly/issues/334

Rudy
-- 
"Chop your own wood and it will warm you twice."
-- Henry Ford; Francis Kinloch, 1819; Henry David Thoreau, 1854

Rudolf Adamkovič <salutis@me.com> [he/him]
Studenohorská 25
84103 Bratislava
Slovakia

Re: org-babel guile source block bug in handling multiple values

[Zelphir Kaltstahl]

[-- Attachment #1: Type: text/plain, Size: 2465 bytes --]

On 3/7/23 20:52, Bruno Barbier wrote:
> Zelphir Kaltstahl<zelphirkaltstahl@posteo.de>  writes:
>
>
>> If org merely wraps in a `let`, it should not notice any of the multiple values
>> business, because that is something done internally in `let-values`.
>>
> The "let", to define the org variables, ends up putting the "import"
> inside the scheme expression, like this:
>
>        ;; -*- geiser-scheme-implementation: guile -*-
>        (let ((x '1)
>              (y '2))
>        (import (except (rnrs base) error vector-map)
>                 (only (guile)
>                       lambda*
>                       λ)
>                 ;; let-values
>                 (srfi srfi-11))
>
>        (let-values ([(a b) (values x y)])
>           (simple-format #t "~a ~a\n" a b))
>        )
>
>
> which raises an error when evaluated the first time (the second time,
> the "import" has already imported the new "let-values" feature
> (srfi-11), so, the evaluation works). You can test this manually in a
> guile session.
>
> I guess you'll have to use sessions, and do the "import" in a separate
> block first.
>
> Bruno

Hello Bruno!

Thank you for that explanation. Behaves exactly like you describe and explains 
things.

I wonder though, whether that let wrapped code should actually cause an error, 
or that is a bug in Guile.

Also thanks for the idea with sessions + separate import source block. I thought 
that should work, but apparently that also has the same error, when running for 
the first time:

~~~~START~~~~
#+name: scheme-time-imports
#+begin_src scheme :eval query-export :noweb strip-export :session scheme-time :results output replace drawer"
(import
  ;; let-values
  (srfi srfi-11))
#+end_src

#+RESULTS: scheme-time-imports

#+name: scheme-time
#+begin_src scheme :eval query-export :noweb strip-export :session scheme-time :results output replace drawer :var x=1 :var y=2
<<scheme-time-imports>>
(import (except (rnrs base) error vector-map)
         (only (guile)
               lambda*
               λ)
         ;; let-values
         (srfi srfi-11))

(let-values ([(a b) (values x y)])
   (simple-format #t "~a ~a\n" a b))
#+end_src

#+RESULTS: scheme-time
:results:
ice-9/boot-9.scm:1685:16: In procedure raise-exception:
Unbound variable: a

Entering a new prompt.  Type `,bt' for a backtrace or `,q' to continue.
scheme@(guile-user) [1]>
:end:
~~~~~END~~~~~

Regards,
Zelphir

-- 
repositories:https://notabug.org/ZelphirKaltstahl

[-- Attachment #2: Type: text/html, Size: 3196 bytes --]

Re: org-babel guile source block bug in handling multiple values

[Zelphir Kaltstahl]

[-- Attachment #1: Type: text/plain, Size: 2868 bytes --]

On 3/7/23 20:52, Bruno Barbier wrote:
> Zelphir Kaltstahl<zelphirkaltstahl@posteo.de>  writes:
>
>
>> If org merely wraps in a `let`, it should not notice any of the multiple values
>> business, because that is something done internally in `let-values`.
>>
> The "let", to define the org variables, ends up putting the "import"
> inside the scheme expression, like this:
>
>        ;; -*- geiser-scheme-implementation: guile -*-
>        (let ((x '1)
>              (y '2))
>        (import (except (rnrs base) error vector-map)
>                 (only (guile)
>                       lambda*
>                       λ)
>                 ;; let-values
>                 (srfi srfi-11))
>
>        (let-values ([(a b) (values x y)])
>           (simple-format #t "~a ~a\n" a b))
>        )
>
>
> which raises an error when evaluated the first time (the second time,
> the "import" has already imported the new "let-values" feature
> (srfi-11), so, the evaluation works). You can test this manually in a
> guile session.
>
> I guess you'll have to use sessions, and do the "import" in a separate
> block first.
>
> Bruno

Actually, now that I think about it, the whole problem is gone, when replacing 
the wrapping let with 2 separate (define ...), which I originally thought org 
would do:

~~~~START~~~~
$ guile
GNU Guile 3.0.9
Copyright (C) 1995-2023 Free Software Foundation, Inc.

Guile comes with ABSOLUTELY NO WARRANTY; for details type `,show w'.
This program is free software, and you are welcome to redistribute it
under certain conditions; type `,show c' for details.

Enter `,help' for help.
scheme@(guile-user)> (define x 1)
scheme@(guile-user)> (define y 2)
scheme@(guile-user)> (import (except (rnrs base) error vector-map)
...         (only (guile)
...               lambda*
...               λ)
...         ;; let-values
...         (srfi srfi-11))
scheme@(guile-user)> (let-values ([(a b) (values x y)])
...   (simple-format #t "~a ~a\n" a b))
1 2
~~~~~END~~~~~

Or in one input to the REPL:

~~~~START~~~~
$ guile
GNU Guile 3.0.9
Copyright (C) 1995-2023 Free Software Foundation, Inc.

Guile comes with ABSOLUTELY NO WARRANTY; for details type `,show w'.
This program is free software, and you are welcome to redistribute it
under certain conditions; type `,show c' for details.

Enter `,help' for help.
scheme@(guile-user)> (define x 1)
scheme@(guile-user)> (define y 2)
scheme@(guile-user)> (import (except (rnrs base) error vector-map)
...         (only (guile)
...               lambda*
...               λ)
...         ;; let-values
...         (srfi srfi-11))
scheme@(guile-user)> (let-values ([(a b) (values x y)])
...   (simple-format #t "~a ~a\n" a b))
1 2
~~~~~END~~~~~

Is there a reason it has to be wrapped in a let, instead of simply define-ing 
the variables?

Regards,
Zelphir

-- 
repositories:https://notabug.org/ZelphirKaltstahl

[-- Attachment #2: Type: text/html, Size: 3652 bytes --]

Re: org-babel guile source block bug in handling multiple values

[Ihor Radchenko]

Zelphir Kaltstahl <zelphirkaltstahl@posteo.de> writes:

> Actually, now that I think about it, the whole problem is gone, when replacing 
> the wrapping let with 2 separate (define ...), which I originally thought org 
> would do:
> ...
> Is there a reason it has to be wrapped in a let, instead of simply define-ing 
> the variables?

Because (define ...) will, AFAIU, define the variables in the whole
session, including subsequent code block calls. Let-binding is safer in
this regard - it only affects a specific code block.

For the problem of (import ...), a similar problem is solved by ob-C
using :includes header argument. See
https://orgmode.org/worg/org-contrib/babel/languages/ob-doc-C.html

I am not yet sure if let-binding around (import ...) must never be done
in Guile. It would be helpful of someone more familiar with Guile chimes
in.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
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>

Re: org-babel guile source block bug in handling multiple values

[Bruno Barbier]

Hi Zelphir,

Zelphir Kaltstahl <zelphirkaltstahl@posteo.de> writes:

> On 3/7/23 20:52, Bruno Barbier wrote:

> Also thanks for the idea with sessions + separate import source block. I thought 
> that should work, but apparently that also has the same error, when running for 
> the first time:
>
> ...

Oh, I see. I tested something way simpler :-)

First, one block to open and configure the guile session.

     #+begin_src scheme :session "!guile" :results silent
     (import (except (rnrs base) error vector-map)
              (only (guile)
                    lambda*
                    λ)
              ;; let-values
              (srfi srfi-11))
     #+end_src

Then, you can get to work and evaluate as many blocks as you like in
that session:

     #+begin_src scheme :session "!guile" :results output replace drawer :var x=1 :var y=2
     (let-values ([(a b) (values x y)])
        (simple-format #t "~a ~a\n" a b))
     #+end_src

     #+RESULTS:
     :results:
     1 2
     :end:

Bruno

Re: org-babel guile source block bug in handling multiple values

[Zelphir Kaltstahl]

[-- Attachment #1: Type: text/plain, Size: 8190 bytes --]

On 3/8/23 20:38, Bruno Barbier wrote:
> Hi Zelphir,
>
> Zelphir Kaltstahl<zelphirkaltstahl@posteo.de>  writes:
>
>> On 3/7/23 20:52, Bruno Barbier wrote:
>> Also thanks for the idea with sessions + separate import source block. I thought
>> that should work, but apparently that also has the same error, when running for
>> the first time:
>>
>> ...
> Oh, I see. I tested something way simpler :-)
>
> First, one block to open and configure the guile session.
>
>       #+begin_src scheme :session "!guile" :results silent
>       (import (except (rnrs base) error vector-map)
>                (only (guile)
>                      lambda*
>                      λ)
>                ;; let-values
>                (srfi srfi-11))
>       #+end_src
>
> Then, you can get to work and evaluate as many blocks as you like in
> that session:
>
>       #+begin_src scheme :session "!guile" :results output replace drawer :var x=1 :var y=2
>       (let-values ([(a b) (values x y)])
>          (simple-format #t "~a ~a\n" a b))
>       #+end_src
>
>       #+RESULTS:
>       :results:
>       1 2
>       :end:
>
> Bruno
Hello Bruno and hello mailing list!

I just tested it a little more:

If there is any (import ...) at all in the code block that makes use of the 
(let-values ...), it seems to somehow disturb already imported libraries. For 
example the following also does not work:

~~~~START~~~~
#+name: scheme-time-imports
#+begin_src scheme :eval query-export :noweb strip-export :session scheme-time :results output replace drawer"
(import
  (except (rnrs base) error vector-map)
  (only (guile)
        lambda*
        λ)
  ;; let-values
  (srfi srfi-11))
#+end_src

#+RESULTS: scheme-time-imports

#+name: scheme-time
#+begin_src scheme :eval query-export :noweb strip-export :session scheme-time :results output replace drawer :var x=1 :var y=2
<<scheme-time-imports>>
(import (srfi srfi-1))
(let-values ([(a b) (values x y)])
    (simple-format #t "~a ~a\n" a b))
#+end_src
~~~~~END~~~~~

So that means, even if the import has nothing to do with the actual import which 
would provide the let-values form, it disturbs it.

I am not sure (let ...) is a correct wrapper for noweb included source blocks. 
What, if I write a (define ...) in my source block and want to use that source 
block via noweb in another source block? Expected behavior I think would be to 
be able to access those variables in other source blocks, since they are defined 
on a top level in an earlier source block, but if they are wrapped in a (let 
...), that would make them only available in the (let ...)? It seems to me, that 
the simple wrapping with a (let ...) might not be the right thing to do. Testing 
that:

~~~~START~~~~
#+name: scheme-defs
#+begin_src scheme :eval query-export :noweb strip-export :session myguile :results output replace drawer :var x=1 :var y=2
(define a x)
(define b y)
#+end_src

#+name: scheme-time
#+begin_src scheme :eval query-export :noweb strip-export :session myguile :results output replace drawer
<<scheme-defs>>
(simple-format #t "~a ~a\n" a b)
#+end_src
~~~~~END~~~~~

Indeed, that also does not work.

I guess I did never hit this problem earlier, because I "oursourced" my imports 
and in imports I do not need any :var header arguments.

I've asked on the Guile IRC channel and something interesting is the case here 
(thanks for clearing it up flatwhatson!) and I understand it as follows:

Imports inside (let ...) work. It is just that let-values is a macro and macros 
are expanded before execution time. However, Guile gets to the body of the 
wrapping (let ...) at execution time. That means, that when Guile gets to 
evaluate the body of the let, it does not expand the let-values, because it is 
already at execution time and no longer at macro expansion time. The import 
might import the let-values form, or might not, but it is already too late to 
expand the (let-values ...).

What is still a bit weird about it is, that in the original example with 
`let-values` I don't get an error about `let-values` not being defined, but only 
about `a` not being defined. And in the example with (define ...) and :var 
above, I get a message about `x` not being defined, instead of `a` not being 
defined.

Probably a good general workaround is to only have imports at the top level, by 
moving them into a source block, which does not have any :var header arguments.

OK, the question is though, whether org should wrap anything in a (let ...) at 
all. During discussion on the Guile IRC, some points against let-wrapping were 
brought up:

(1) The presence of a :var header argument currently determines, whether the 
code in the source block is wrapped with a (let ...). One argument for that was, 
that this way the variables do not leak. But this also decides, whether other 
things leak. For example (import ...) or (define ...). Should :var decide, 
whether bindings created with (define ...) are visible in other source blocks 
including the source block with the :var header arguments? It seems like a 
responsibility :var should not have and definitely is unexpected for the user.

(2) Wrapping in a (let ...) also moves things from the top level to the inside 
of expressions. This can make them move from one phase to another. Phases such 
as described here: 
https://www.gnu.org/software/guile/manual/html_node/Eval-When.html. This is 
probably not intended and also unexpected.

(3) Not wrapping with a (let ...) expression and instead going with simple 
(define ...) for :var variables seems a very general solution, that should be 
portable to all Schemes.

(4) To make things easily understandable, bindings defined in a source block 
that is included in another source block, should either always leak, or never. 
Not sometimes this way and sometimes that way. But conditionally wrapping with 
(let ...) if there is a :var header argument introduces a behavior that changes 
that.

(5) Imagine you already have a finished document and want to extend it, by 
letting one of the source blocks take a variable via :var header argument. Now 
you must change the source block as well, and not only place the variable's name 
somewhere, but also need to take care of imports and evaluation and expansion 
phases.

(6) Looking at other languages and how it behaves there. Say Python for example, 
there are no (let ...) expressions available, so no let-wrapping could happen. 
Unless org invents an additional `def` to wrap it in that, but only if there are 
:var header arguments. Lets see how things work there:

~~~~START~~~~
#+name: python-imports
#+begin_src python :python /usr/bin/python3 :results output replace drawer :var x=4
import math

y = math.sqrt(x)
# print(y)
#+end_src

#+name: python-usage
#+begin_src python :python /usr/bin/python3 :return :noweb strip-export :results value replace drawer
<<python-imports>>

print("y: {}".format(y))
#+end_src
~~~~~END~~~~~

Unfortunately, this example does not seem to work at all, but for a different 
reason:

It seems that using any Python source block with :var header args via :noweb 
does not work, as it then behaves in the way, that it merely pasted the included 
source block, without first putting in the :var values into the variables. I get 
errors about those :var variables being undefined, of course, since they are on 
the included source block, not on the including one:

~~~~START: *Org-Babel Error Output*~~~~
Traceback (most recent call last):
   File "<stdin>", line 10, in <module>
   File "<stdin>", line 5, in main
NameError: name 'x' is not defined
[ Babel evaluation exited with code 1 ]
~~~~~END~~~~~

So it would seem, that :var does not work with :noweb for Python source blocks yet.

(7) As an idea, one could still invent a new header argument that determines 
whether to let-wrap or not, if it is really needed for any other Scheme.

OK, to wrap up (ha!), I want to ask:

(q1) What is a rationale, if any, behind the let-wrapping?

(q2) Any chances of that changing to (define ...)?

(q3) How could I change my org-mode's code to not  let-wrap, and instead use 
(define ...)?

Best regards,
Zelphir

-- 
repositories:https://notabug.org/ZelphirKaltstahl

[-- Attachment #2: Type: text/html, Size: 9860 bytes --]

[BUG] Inconsistent global/local :var assignments in ob-* for lisps and non-lisps (was: org-babel guile source block bug in handling multiple values)

[Ihor Radchenko]

Zelphir Kaltstahl <zelphirkaltstahl@posteo.de> writes:

> I am not sure (let ...) is a correct wrapper for noweb included source blocks. 
> What, if I write a (define ...) in my source block and want to use that source 
> block via noweb in another source block? Expected behavior I think would be to 
> be able to access those variables in other source blocks, since they are defined 
> on a top level in an earlier source block, but if they are wrapped in a (let 
> ...), that would make them only available in the (let ...)? It seems to me, that 
> the simple wrapping with a (let ...) might not be the right thing to do. Testing 
> that:
>
> ~~~~START~~~~
> #+name: scheme-defs
> #+begin_src scheme :eval query-export :noweb strip-export :session myguile :results output replace drawer :var x=1 :var y=2
> (define a x)
> (define b y)
> #+end_src
>
> #+name: scheme-time
> #+begin_src scheme :eval query-export :noweb strip-export :session myguile :results output replace drawer
> <<scheme-defs>>
> (simple-format #t "~a ~a\n" a b)
> #+end_src
> ~~~~~END~~~~~
>
> Indeed, that also does not work.

I just checked ob-C, ob-shell, ob-emacs-lisp, and ob-clojure.
Non-lisps appear to assign the values globally.
In contrast, all the lisp babel backends are using let-bindings.

Considering the existing inconsistency, and the raised bug I'd be in
favor of making variable assignments global in all the lisp babel
backends.

The only possible exception is ob-emacs-lisp. Executing elisp code is
done in current Elisp session and thus using global variable assignments
may be tricky. Unless we juggle with multiple obarrays.

> I guess I did never hit this problem earlier, because I "oursourced" my imports 
> and in imports I do not need any :var header arguments.
>
> I've asked on the Guile IRC channel and something interesting is the case here 
> (thanks for clearing it up flatwhatson!) and I understand it as follows:
>
> Imports inside (let ...) work. It is just that let-values is a macro and macros 
> are expanded before execution time. However, Guile gets to the body of the 
> wrapping (let ...) at execution time. That means, that when Guile gets to 
> evaluate the body of the let, it does not expand the let-values, because it is 
> already at execution time and no longer at macro expansion time. The import 
> might import the let-values form, or might not, but it is already too late to 
> expand the (let-values ...).

So, apparently using `let' is not universally safe in Guile.

> OK, the question is though, whether org should wrap anything in a (let ...) at 
> all. During discussion on the Guile IRC, some points against let-wrapping were 
> brought up:
>
> (1) The presence of a :var header argument currently determines, whether the 
> code in the source block is wrapped with a (let ...). One argument for that was, 
> that this way the variables do not leak. But this also decides, whether other 
> things leak. For example (import ...) or (define ...). Should :var decide, 
> whether bindings created with (define ...) are visible in other source blocks 
> including the source block with the :var header arguments? It seems like a 
> responsibility :var should not have and definitely is unexpected for the user.

This is something Guile-specific. In Elisp, let-binding still allows
`defun' or `defvar'.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
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>

Re: org-babel guile source block bug in handling multiple values

[Ihor Radchenko]

Zelphir Kaltstahl <zelphirkaltstahl@posteo.de> writes:

> ~~~~START~~~~
> #+name: python-imports
> #+begin_src python :python /usr/bin/python3 :results output replace drawer :var x=4
> import math
>
> y = math.sqrt(x)
> # print(y)
> #+end_src
>
> #+name: python-usage
> #+begin_src python :python /usr/bin/python3 :return :noweb strip-export :results value replace drawer
> <<python-imports>>
>
> print("y: {}".format(y))
> #+end_src
> ~~~~~END~~~~~
>
> Unfortunately, this example does not seem to work at all, but for a different 
> reason:
>
> It seems that using any Python source block with :var header args via :noweb 
> does not work, as it then behaves in the way, that it merely pasted the included 
> source block, without first putting in the :var values into the variables. I get 
> errors about those :var variables being undefined, of course, since they are on 
> the included source block, not on the including one:
>
> ~~~~START: *Org-Babel Error Output*~~~~
> Traceback (most recent call last):
>    File "<stdin>", line 10, in <module>
>    File "<stdin>", line 5, in main
> NameError: name 'x' is not defined
> [ Babel evaluation exited with code 1 ]
> ~~~~~END~~~~~

This is expected. Noweb includes the src block code without altering it.
See 16.11 Noweb Reference Syntax

We may probably clarify this in the manual. Would it be helpful?

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
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>

Re: org-babel guile source block bug in handling multiple values

[Ihor Radchenko]

Zelphir Kaltstahl <zelphirkaltstahl@posteo.de> writes:

> OK, to wrap up (ha!), I want to ask:
>
> (q1) What is a rationale, if any, behind the let-wrapping?

It makes sense in ob-emacs-lisp to not litter global Emacs state.
In other ob-* lisp backends, I am not sure.
I am CCing Daniel, the maintainer of ob-clojure (we have no active
maintainer for ob-scheme now). Maybe, Daniel has some useful insight.

> (q2) Any chances of that changing to (define ...)?

This sounds reasonable.

> (q3) How could I change my org-mode's code to not  let-wrap, and instead use 
> (define ...)?

See `org-babel-expand-body:scheme'. You can re-define it for a quick
temporary fix.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
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>

Re: org-babel guile source block bug in handling multiple values

[Daniel Kraus]

Ihor Radchenko <yantar92@posteo.net> writes:

> Zelphir Kaltstahl <zelphirkaltstahl@posteo.de> writes:

>> (q1) What is a rationale, if any, behind the let-wrapping?
>
> It makes sense in ob-emacs-lisp to not litter global Emacs state.
> In other ob-* lisp backends, I am not sure.
> I am CCing Daniel, the maintainer of ob-clojure (we have no active
> maintainer for ob-scheme now). Maybe, Daniel has some useful insight.

No, unfortunately I don't have any more insights than already discussed.

I think wrapping the vars in let just seems natural for lisps.

If I could freely choose if a :var declaration in one source block
should create a global variable for all other blocks in this session,
I would say making it only available in the defining source block
is more natural (i.e. use let instead of def).
But given that apparently almost all other babel languages define
a global var, I'll just make the same change in ob-clojure?!

Cheers,
  Daniel

Re: [BUG] Inconsistent global/local :var assignments in ob-* for lisps and non-lisps (was: org-babel guile source block bug in handling multiple values)

[Zelphir Kaltstahl]

On 3/9/23 14:04, Ihor Radchenko wrote:
> Zelphir Kaltstahl <zelphirkaltstahl@posteo.de> writes:
>
>> I am not sure (let ...) is a correct wrapper for noweb included source blocks.
>> What, if I write a (define ...) in my source block and want to use that source
>> block via noweb in another source block? Expected behavior I think would be to
>> be able to access those variables in other source blocks, since they are defined
>> on a top level in an earlier source block, but if they are wrapped in a (let
>> ...), that would make them only available in the (let ...)? It seems to me, that
>> the simple wrapping with a (let ...) might not be the right thing to do. Testing
>> that:
>>
>> ~~~~START~~~~
>> #+name: scheme-defs
>> #+begin_src scheme :eval query-export :noweb strip-export :session myguile :results output replace drawer :var x=1 :var y=2
>> (define a x)
>> (define b y)
>> #+end_src
>>
>> #+name: scheme-time
>> #+begin_src scheme :eval query-export :noweb strip-export :session myguile :results output replace drawer
>> <<scheme-defs>>
>> (simple-format #t "~a ~a\n" a b)
>> #+end_src
>> ~~~~~END~~~~~
>>
>> Indeed, that also does not work.
> I just checked ob-C, ob-shell, ob-emacs-lisp, and ob-clojure.
> Non-lisps appear to assign the values globally.
> In contrast, all the lisp babel backends are using let-bindings.
>
> Considering the existing inconsistency, and the raised bug I'd be in
> favor of making variable assignments global in all the lisp babel
> backends.
>
> The only possible exception is ob-emacs-lisp. Executing elisp code is
> done in current Elisp session and thus using global variable assignments
> may be tricky. Unless we juggle with multiple obarrays.
>
>> I guess I did never hit this problem earlier, because I "oursourced" my imports
>> and in imports I do not need any :var header arguments.
>>
>> I've asked on the Guile IRC channel and something interesting is the case here
>> (thanks for clearing it up flatwhatson!) and I understand it as follows:
>>
>> Imports inside (let ...) work. It is just that let-values is a macro and macros
>> are expanded before execution time. However, Guile gets to the body of the
>> wrapping (let ...) at execution time. That means, that when Guile gets to
>> evaluate the body of the let, it does not expand the let-values, because it is
>> already at execution time and no longer at macro expansion time. The import
>> might import the let-values form, or might not, but it is already too late to
>> expand the (let-values ...).
> So, apparently using `let' is not universally safe in Guile.
Is it safe in any Scheme? I think that depends on expansion and evaluation 
phases of the Scheme standards (see below).
>> OK, the question is though, whether org should wrap anything in a (let ...) at
>> all. During discussion on the Guile IRC, some points against let-wrapping were
>> brought up:
>>
>> (1) The presence of a :var header argument currently determines, whether the
>> code in the source block is wrapped with a (let ...). One argument for that was,
>> that this way the variables do not leak. But this also decides, whether other
>> things leak. For example (import ...) or (define ...). Should :var decide,
>> whether bindings created with (define ...) are visible in other source blocks
>> including the source block with the :var header arguments? It seems like a
>> responsibility :var should not have and definitely is unexpected for the user.
> This is something Guile-specific. In Elisp, let-binding still allows
> `defun' or `defvar'.

The issue is not with defining via (define ...) inside a (let ...) in Guile. It 
is about importing macros at the time, when the body of the (let ...) is already 
evaluated, which is at a later phase than macro expansion. By wrapping inside a 
(let ...) org has moved the import to a later phase, which causes the macro 
(let-values ...) to not be expanded.

As far as I know, (defun ...) and (defvar ...) are merely defining functions and 
variables, not macros.

My point is, that imports are usually global for sessions. But :var decided for 
let-wrapping, moving them to a different place. Just like imports are usually 
global, I would expect (define ...)s to be global in the session, unless I put 
them inside a narrowed scope like a (let ...) myself. The org generated (let 
...) is invisible to the user and thus confusing, at least for GNU Guile.

For other Schemes it probably all depends on how their phases of expansion and 
evaluation work. I don't know enough about the Scheme standards, to tell, 
whether Guile has the correct behavior here or whether there is a correct 
behavior defined in the Scheme standards. Maybe someone more knowledgeable can 
chime in to comment on that.

-- 
repositories: https://notabug.org/ZelphirKaltstahl

Re: org-babel guile source block bug in handling multiple values

[Zelphir Kaltstahl]

On 3/9/23 14:10, Ihor Radchenko wrote:
> Zelphir Kaltstahl <zelphirkaltstahl@posteo.de> writes:
>
>> ~~~~START~~~~
>> #+name: python-imports
>> #+begin_src python :python /usr/bin/python3 :results output replace drawer :var x=4
>> import math
>>
>> y = math.sqrt(x)
>> # print(y)
>> #+end_src
>>
>> #+name: python-usage
>> #+begin_src python :python /usr/bin/python3 :return :noweb strip-export :results value replace drawer
>> <<python-imports>>
>>
>> print("y: {}".format(y))
>> #+end_src
>> ~~~~~END~~~~~
>>
>> Unfortunately, this example does not seem to work at all, but for a different
>> reason:
>>
>> It seems that using any Python source block with :var header args via :noweb
>> does not work, as it then behaves in the way, that it merely pasted the included
>> source block, without first putting in the :var values into the variables. I get
>> errors about those :var variables being undefined, of course, since they are on
>> the included source block, not on the including one:
>>
>> ~~~~START: *Org-Babel Error Output*~~~~
>> Traceback (most recent call last):
>>     File "<stdin>", line 10, in <module>
>>     File "<stdin>", line 5, in main
>> NameError: name 'x' is not defined
>> [ Babel evaluation exited with code 1 ]
>> ~~~~~END~~~~~
> This is expected. Noweb includes the src block code without altering it.
> See 16.11 Noweb Reference Syntax
>
> We may probably clarify this in the manual. Would it be helpful?
I think it would be helpful. I merely tried Python, because it was at hand and 
because I did not find ob-racket in M-x package-list RET and did not have 
another Scheme installed. Cannot say, whether others find the behavior confusing 
or not, but I imagine I would find it helpful, if it was mentioned in the docs.

-- 
repositories: https://notabug.org/ZelphirKaltstahl

Re: org-babel guile source block bug in handling multiple values

[Zelphir Kaltstahl]

[-- Attachment #1: Type: text/plain, Size: 1858 bytes --]


On 3/9/23 14:11, Ihor Radchenko wrote:
> Zelphir Kaltstahl<zelphirkaltstahl@posteo.de>  writes:
>
>> OK, to wrap up (ha!), I want to ask:
>>
>> (q1) What is a rationale, if any, behind the let-wrapping?
> It makes sense in ob-emacs-lisp to not litter global Emacs state.
> In other ob-* lisp backends, I am not sure.
> I am CCing Daniel, the maintainer of ob-clojure (we have no active
> maintainer for ob-scheme now). Maybe, Daniel has some useful insight.
>
>> (q2) Any chances of that changing to (define ...)?
> This sounds reasonable.
>
>> (q3) How could I change my org-mode's code to not  let-wrap, and instead use
>> (define ...)?
> See `org-babel-expand-body:scheme'. You can re-define it for a quick
> temporary fix.

Thanks for the hint!

Here is my fix for my init.el:

~~~~START~~~~
;; Redefine/override org-babel-expand-body:scheme to avoid
;; let-wrapping with :var header args in source blocks.

(defun xiaolong/org-babel-format-vars (vars)
   "Format :var header arguments given as VARS."
   (mapconcat (lambda (var)
                (format "(define %s %S)" (car var) (cdr var)))
              vars
              "\n"))

(eval-after-load "org"
   '(defun org-babel-expand-body:scheme (body params)
      "Expand BODY according to PARAMS, return the expanded body."
      (let ((vars (org-babel--get-vars params))
            (prepends (cdr (assq :prologue params)))
            (postpends (cdr (assq :epilogue params))))
        (concat (and prepends (concat prepends "\n"))
                (if (null vars)
                    body
                  (concat (xiaolong/org-babel-format-vars vars) body))
                (and postpends (concat "\n" postpends))))))
~~~~~END~~~~~

That was simpler than I expected and the first time I overrode anything of 
org-mode =)

Regards,
Zelphir

-- 
repositories:https://notabug.org/ZelphirKaltstahl

[-- Attachment #2: Type: text/html, Size: 2872 bytes --]

Re: org-babel guile source block bug in handling multiple values

[Ihor Radchenko]

Daniel Kraus <daniel@kraus.my> writes:

> If I could freely choose if a :var declaration in one source block
> should create a global variable for all other blocks in this session,
> I would say making it only available in the defining source block
> is more natural (i.e. use let instead of def).

You can always introduce a language-specific header argument that will
control this behaviour. If you think that it can be useful.

Note that org-babel gives no promises about :vars being globally
available in :session. So, there is technically no obligation to prefer
global definition over let-binding. Unless let-binding can cause bugs,
as the one reported here for Guile.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
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>

Re: [BUG] Inconsistent global/local :var assignments in ob-* for lisps and non-lisps (was: org-babel guile source block bug in handling multiple values)

[Ihor Radchenko]

Zelphir Kaltstahl <zelphirkaltstahl@posteo.de> writes:

> The issue is not with defining via (define ...) inside a (let ...) in Guile. It 
> is about importing macros at the time, when the body of the (let ...) is already 
> evaluated, which is at a later phase than macro expansion. By wrapping inside a 
> (let ...) org has moved the import to a later phase, which causes the macro 
> (let-values ...) to not be expanded.

I see.
AFAIK, Elisp does not have this problem.

> As far as I know, (defun ...) and (defvar ...) are merely defining functions and 
> variables, not macros.

Same for defmacro in Elisp.

> My point is, that imports are usually global for sessions. But :var decided for 
> let-wrapping, moving them to a different place. Just like imports are usually 
> global, I would expect (define ...)s to be global in the session, unless I put 
> them inside a narrowed scope like a (let ...) myself. The org generated (let 
> ...) is invisible to the user and thus confusing, at least for GNU Guile.
>
> For other Schemes it probably all depends on how their phases of expansion and 
> evaluation work. I don't know enough about the Scheme standards, to tell, 
> whether Guile has the correct behavior here or whether there is a correct 
> behavior defined in the Scheme standards. Maybe someone more knowledgeable can 
> chime in to comment on that.

When saying Guile I mean scheme. Remember that I am now looking from a
more general perspective of other ob-* libraries.

My conclusion so far is that it is not safe in ob-scheme to use
let-binding. Other ob-* lisp implementations may be OK (at least,
ob-emacs-lisp is OK).

Now, the main question is whether it is safe to use `define' in all the
scheme implementations. If it is, would you be interested in turning
your personal fix into a patch for ob-scheme?

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
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>

Re: org-babel guile source block bug in handling multiple values

[Ihor Radchenko]

[-- Attachment #1: Type: text/plain, Size: 600 bytes --]

Zelphir Kaltstahl <zelphirkaltstahl@posteo.de> writes:

>> This is expected. Noweb includes the src block code without altering it.
>> See 16.11 Noweb Reference Syntax
>>
>> We may probably clarify this in the manual. Would it be helpful?
> I think it would be helpful. I merely tried Python, because it was at hand and 
> because I did not find ob-racket in M-x package-list RET and did not have 
> another Scheme installed. Cannot say, whether others find the behavior confusing 
> or not, but I imagine I would find it helpful, if it was mentioned in the docs.

See the attached tentative patch.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org-manual.org-Explain-that-noweb-expansion-does-not.patch --]
[-- Type: text/x-patch, Size: 1800 bytes --]

From 26763df7de8f742e76f10f2e7603ed290b97df65 Mon Sep 17 00:00:00 2001
Message-Id: <26763df7de8f742e76f10f2e7603ed290b97df65.1678529850.git.yantar92@posteo.net>
From: Ihor Radchenko <yantar92@posteo.net>
Date: Sat, 11 Mar 2023 11:16:23 +0100
Subject: [PATCH] org-manual.org: Explain that noweb expansion does not carry
 over :var

* doc/org-manual.org (Noweb Reference Syntax): Provide an example
explaining that :var header arguments are not in effect when expanding
noweb reference.

Reported-by: Zelphir Kaltstahl <zelphirkaltstahl@posteo.de>
Link: https://orgmode.org/list/46e6f579-9eca-e1da-06ea-f2478a603c5a@posteo.de
---
 doc/org-manual.org | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/doc/org-manual.org b/doc/org-manual.org
index 1c97d6aa8..2c3ec46a4 100644
--- a/doc/org-manual.org
+++ b/doc/org-manual.org
@@ -19117,6 +19117,32 @@ ** Noweb Reference Syntax
 ,#+END_SRC
 #+end_example
 
+Note that noweb expansion does not automatically carry over =var=
+header arguments.  In the following example, attempting to evaluate
+the second code block will give an error, because the variables
+defined in the first code block will not be defined in the second
+block.
+
+#+begin_example
+,#+NAME: get-prompt
+,#+BEGIN_SRC emacs-lisp :var prompt="root> " :var command="ls"
+  (concat prompt command)
+,#+END_SRC
+
+,#+RESULTS: get-prompt
+: root> ls
+
+,#+BEGIN_SRC emacs-lisp :noweb yes
+  <<get-prompt>>
+,#+END_SRC
+
+The previous block is expanded to
+
+,#+BEGIN_SRC emacs-lisp
+  (concat prompt command)
+,#+END_SRC
+#+end_example
+
 You may also include the contents of multiple blocks sharing a common
 =noweb-ref= header argument, which can be set at the file, subtree,
 or code block level.  In the example Org file shown next, the body of
-- 
2.39.1


[-- Attachment #3: Type: text/plain, Size: 224 bytes --]


-- 
Ihor Radchenko // yantar92,
Org mode contributor,
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>

Re: [BUG] Inconsistent global/local :var assignments in ob-* for lisps and non-lisps (was: org-babel guile source block bug in handling multiple values)

[Zelphir Kaltstahl]

On 3/11/23 10:58, Ihor Radchenko wrote:
> Zelphir Kaltstahl <zelphirkaltstahl@posteo.de> writes:
>
>> The issue is not with defining via (define ...) inside a (let ...) in Guile. It
>> is about importing macros at the time, when the body of the (let ...) is already
>> evaluated, which is at a later phase than macro expansion. By wrapping inside a
>> (let ...) org has moved the import to a later phase, which causes the macro
>> (let-values ...) to not be expanded.
> I see.
> AFAIK, Elisp does not have this problem.
>
>> As far as I know, (defun ...) and (defvar ...) are merely defining functions and
>> variables, not macros.
> Same for defmacro in Elisp.
>
>> My point is, that imports are usually global for sessions. But :var decided for
>> let-wrapping, moving them to a different place. Just like imports are usually
>> global, I would expect (define ...)s to be global in the session, unless I put
>> them inside a narrowed scope like a (let ...) myself. The org generated (let
>> ...) is invisible to the user and thus confusing, at least for GNU Guile.
>>
>> For other Schemes it probably all depends on how their phases of expansion and
>> evaluation work. I don't know enough about the Scheme standards, to tell,
>> whether Guile has the correct behavior here or whether there is a correct
>> behavior defined in the Scheme standards. Maybe someone more knowledgeable can
>> chime in to comment on that.
> When saying Guile I mean scheme. Remember that I am now looking from a
> more general perspective of other ob-* libraries.
>
> My conclusion so far is that it is not safe in ob-scheme to use
> let-binding. Other ob-* lisp implementations may be OK (at least,
> ob-emacs-lisp is OK).
>
> Now, the main question is whether it is safe to use `define' in all the
> scheme implementations. If it is, would you be interested in turning
> your personal fix into a patch for ob-scheme?

Sure! Would need to familiarize myself with with process of doing that, but why not.

I guess it would be a safer bet to await, whether the patch is what the general 
solution should be. Or would a patch be good to have, regardless of whether the 
official implementation changes or not, so that others can apply it perhaps, 
instead of putting something in their personal init file?

-- 
repositories: https://notabug.org/ZelphirKaltstahl

Re: [BUG] Inconsistent global/local :var assignments in ob-* for lisps and non-lisps (was: org-babel guile source block bug in handling multiple values)

[Ihor Radchenko]

Zelphir Kaltstahl <zelphirkaltstahl@posteo.de> writes:

>> Now, the main question is whether it is safe to use `define' in all the
>> scheme implementations. If it is, would you be interested in turning
>> your personal fix into a patch for ob-scheme?
>
> Sure! Would need to familiarize myself with with process of doing that, but why not.

See https://orgmode.org/worg/org-contribute.html

> I guess it would be a safer bet to await, whether the patch is what the general 
> solution should be. Or would a patch be good to have, regardless of whether the 
> official implementation changes or not, so that others can apply it perhaps, 
> instead of putting something in their personal init file?

I do not expect problems with `define'.
Just if you can, try to test with alternative implementations.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
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>

[PATCH] lisp/ob-scheme.el

[Zelphir Kaltstahl]

[-- Attachment #1: Type: text/plain, Size: 2460 bytes --]

On 3/11/23 10:58, Ihor Radchenko wrote:
> Zelphir Kaltstahl <zelphirkaltstahl@posteo.de> writes:
>
>> The issue is not with defining via (define ...) inside a (let ...) in Guile. It
>> is about importing macros at the time, when the body of the (let ...) is already
>> evaluated, which is at a later phase than macro expansion. By wrapping inside a
>> (let ...) org has moved the import to a later phase, which causes the macro
>> (let-values ...) to not be expanded.
> I see.
> AFAIK, Elisp does not have this problem.
>
>> As far as I know, (defun ...) and (defvar ...) are merely defining functions and
>> variables, not macros.
> Same for defmacro in Elisp.
>
>> My point is, that imports are usually global for sessions. But :var decided for
>> let-wrapping, moving them to a different place. Just like imports are usually
>> global, I would expect (define ...)s to be global in the session, unless I put
>> them inside a narrowed scope like a (let ...) myself. The org generated (let
>> ...) is invisible to the user and thus confusing, at least for GNU Guile.
>>
>> For other Schemes it probably all depends on how their phases of expansion and
>> evaluation work. I don't know enough about the Scheme standards, to tell,
>> whether Guile has the correct behavior here or whether there is a correct
>> behavior defined in the Scheme standards. Maybe someone more knowledgeable can
>> chime in to comment on that.
> When saying Guile I mean scheme. Remember that I am now looking from a
> more general perspective of other ob-* libraries.
>
> My conclusion so far is that it is not safe in ob-scheme to use
> let-binding. Other ob-* lisp implementations may be OK (at least,
> ob-emacs-lisp is OK).
>
> Now, the main question is whether it is safe to use `define' in all the
> scheme implementations. If it is, would you be interested in turning
> your personal fix into a patch for ob-scheme?

Hi!

I've created a patch, which I will attach to this e-mail.

Not sure it meets all formalities. For example it is not clear to me, whether I 
should add the "TINYCHANGE" at the bottom of my commit message.

Still need to get around to test at least some other Scheme as well, but I guess 
I should get started with the patch, otherwise I will procrastinate or be stuck 
in fear of formalities forever.

Let me know, if this an OK patch or what else needs to be done or what format is 
wrong, if any.

-- 
repositories: https://notabug.org/ZelphirKaltstahl

[-- Attachment #2: 0001-lisp-ob-scheme.el.patch --]
[-- Type: text/x-patch, Size: 2204 bytes --]

From 51b299aa18e882681dd681acb51c9cb1b44f3b4e Mon Sep 17 00:00:00 2001
From: Zelphir Kaltstahl <zelphirkaltstahl@posteo.de>
Date: Sat, 18 Mar 2023 16:06:05 +0100
Subject: [PATCH] lisp/ob-scheme.el:

* ob-scheme.el (org-babel-expand-body:scheme,
org-babel-expand-header-arg-vars:scheme): Change the way header
argument :var variables are expanded for for Scheme source blocks.  Use
`define' instead of wrapping using `let'.

Wrapping binding definitions using `let' can lead to issues with GNU
Guile and potentially other Scheme dialects. GNU Guile will only get
to the body of the let at evaluation time, not at macro expansion
time. If the let form wraps any imports of libraries that define
macros, then those imported macros are seen too late and their
corresponding forms inside the body of the let are not
expanded. Using `define' to define bindings avoids this problem, at
least in GNU Guile.
---
 lisp/ob-scheme.el | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/lisp/ob-scheme.el b/lisp/ob-scheme.el
index 9f12f42cb..f837dedd2 100644
--- a/lisp/ob-scheme.el
+++ b/lisp/ob-scheme.el
@@ -79,6 +79,14 @@
 (defvar org-babel-default-header-args:scheme '()
   "Default header arguments for scheme code blocks.")
 
+(defun org-babel-expand-header-arg-vars:scheme (vars)
+  "Expand :var header arguments given as VARS."
+  (mapconcat
+   (lambda (var)
+     (format "(define %s %S)" (car var) (cdr var)))
+   vars
+   "\n"))
+
 (defun org-babel-expand-body:scheme (body params)
   "Expand BODY according to PARAMS, return the expanded body."
   (let ((vars (org-babel--get-vars params))
@@ -86,16 +94,9 @@
 	(postpends (cdr (assq :epilogue params))))
     (concat (and prepends (concat prepends "\n"))
 	    (if (null vars) body
-	      (format "(let (%s)\n%s\n)"
-		      (mapconcat
-		       (lambda (var)
-			 (format "%S" (print `(,(car var) ',(cdr var)))))
-		       vars
-		       "\n      ")
-		      body))
+	      (concat (org-babel-expand-header-arg-vars:scheme vars) body))
 	    (and postpends (concat "\n" postpends)))))
 
-
 (defvar org-babel-scheme-repl-map (make-hash-table :test #'equal)
   "Map of scheme sessions to session names.")
 
-- 
2.25.1

Re: [PATCH] lisp/ob-scheme.el

[Ihor Radchenko]

Zelphir Kaltstahl <zelphirkaltstahl@posteo.de> writes:

> I've created a patch, which I will attach to this e-mail.

Thanks!

> Not sure it meets all formalities. For example it is not clear to me, whether I 
> should add the "TINYCHANGE" at the bottom of my commit message.

You should, unless you have FSF copyright assignment.

> Let me know, if this an OK patch or what else needs to be done or what format is 
> wrong, if any.

See my (minor) comments below.

> -- 
> repositories: https://notabug.org/ZelphirKaltstahl
> From 51b299aa18e882681dd681acb51c9cb1b44f3b4e Mon Sep 17 00:00:00 2001
> From: Zelphir Kaltstahl <zelphirkaltstahl@posteo.de>
> Date: Sat, 18 Mar 2023 16:06:05 +0100
> Subject: [PATCH] lisp/ob-scheme.el:

Please provide a short commit summary, not just the changed file.
See how we do it in https://git.savannah.gnu.org/cgit/emacs/org-mode.git/log/

> Wrapping binding definitions using `let' can lead to issues with GNU
> Guile and potentially other Scheme dialects. GNU Guile will only get
> to the body of the let at evaluation time, not at macro expansion
> time. If the let form wraps any imports of libraries that define
> macros, then those imported macros are seen too late and their
> corresponding forms inside the body of the let are not
> expanded. Using `define' to define bindings avoids this problem, at
> least in GNU Guile.

Please use double space between sentences. Also, it would be helpful to
provide a link to this thread for more context. (The aim of commit
message is a note for future contributors on the reason the change was
made).

> +(defun org-babel-expand-header-arg-vars:scheme (vars)

Please use org-babel-scheme-... function name. It is the usual
Elisp convention to prefix the functions as
library-name-inner-function-name.

The exception in org-babel is a set of special functions that must have
certain name pattern. Expanding header args is not one of those special
functions.

> +  "Expand :var header arguments given as VARS."
> +  (mapconcat
> +   (lambda (var)
> +     (format "(define %s %S)" (car var) (cdr var)))

Is there any reason why you use %s for variable name? Previously it was
formatted with escapes (using %S).

Also, previous version quoted the variable value with "'". Why didn't
you do it here?

> +	      (concat (org-babel-expand-header-arg-vars:scheme vars) body))

`mapconcat' you used in `org-babel-expand-header-arg-vars:scheme' does
not add trailing newline, unlike done previously.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
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>

Re: [PATCH] lisp/ob-scheme.el

[Zelphir Kaltstahl]

[-- Attachment #1.1: Type: text/plain, Size: 6020 bytes --]

>> Not sure it meets all formalities. For example it is not clear to me, whether I
>> should add the "TINYCHANGE" at the bottom of my commit message.
> You should, unless you have FSF copyright assignment.
I am not sure what "have FSF copyright assignment" means. I would not mind 
assigning copyright of that patch to FSF, but probably the paperwork would be 
way overblown for such a small change. I will simply add the "TINYCHANGE" then.
>> -- 
>> repositories:https://notabug.org/ZelphirKaltstahl
>>  From 51b299aa18e882681dd681acb51c9cb1b44f3b4e Mon Sep 17 00:00:00 2001
>> From: Zelphir Kaltstahl<zelphirkaltstahl@posteo.de>
>> Date: Sat, 18 Mar 2023 16:06:05 +0100
>> Subject: [PATCH] lisp/ob-scheme.el:
> Please provide a short commit summary, not just the changed file.
> See how we do it inhttps://git.savannah.gnu.org/cgit/emacs/org-mode.git/log/
Oh, I thought I had one. Not sure how I lost my commit message title. o.o

This is the kind of stuff, that causes me to procrastinate so much. I know it is 
not your fault and I know there need to be some format regulations in place to 
have a good manageable cooperation when working on a community project. I just 
want to explain, why I am so slow in following up and initially sending the 
patch ; ) Not blaming anyone!

>> Wrapping binding definitions using `let' can lead to issues with GNU
>> Guile and potentially other Scheme dialects. GNU Guile will only get
>> to the body of the let at evaluation time, not at macro expansion
>> time. If the let form wraps any imports of libraries that define
>> macros, then those imported macros are seen too late and their
>> corresponding forms inside the body of the let are not
>> expanded. Using `define' to define bindings avoids this problem, at
>> least in GNU Guile.
> Please use double space between sentences. Also, it would be helpful to
> provide a link to this thread for more context. (The aim of commit
> message is a note for future contributors on the reason the change was
> made).
Right! I forgot about that one while writing : ) Will do!
>> +(defun org-babel-expand-header-arg-vars:scheme (vars)
> Please use org-babel-scheme-... function name. It is the usual
> Elisp convention to prefix the functions as
> library-name-inner-function-name.
>
> The exception in org-babel is a set of special functions that must have
> certain name pattern. Expanding header args is not one of those special
> functions.
Ah, I was not aware of that. Took all naming inspiration from the 
org-babel-expand-body:scheme function name and thought my separate function 
ought to have :scheme suffix : ) Will fix!
>> +  "Expand :var header arguments given as VARS."
>> +  (mapconcat
>> +   (lambda (var)
>> +     (format "(define %s %S)" (car var) (cdr var)))
> Is there any reason why you use %s for variable name? Previously it was
> formatted with escapes (using %S).

That was me thinking: "The name of the variable should just be itself, not 
wrapped in double quotes, because in Scheme I cannot create a variable as 
(define "abc" 123)". But maybe I misunderstood %s and %S. I also do not know, 
how elisp's `print' treats its arguments. Will use 2 times %S then.

Or do you mean, that I should be using `print'? I thought using only `format' is 
simpler, so I did not bother with `print' and then `format'. If I choose the 
correct format modifiers, `print' should be unnecessary, right? Or does `print' 
do some magic behind the scenes, that is not expressable using merely format 
modifiers?

> Also, previous version quoted the variable value with "'". Why didn't
> you do it here?

I am not sure I understand what you are referring to in the previous version. Do 
you mean that `print' quoted variable values with a single quote? Do you mean 
this part of the previous code:

(print `(,(car var) ',(cdr var)))

?

>> +	      (concat (org-babel-expand-header-arg-vars:scheme vars) body))
> `mapconcat' you used in `org-babel-expand-header-arg-vars:scheme' does
> not add trailing newline, unlike done previously.

Am I not adding a newline? I think I do?:

~~~~
...

(mapconcat
    (lambda (var)
      (format "(define %S %S)" (car var) (cdr var)))
    vars
    "\n")

...
~~~~

Is anything else required? Maybe 2 newlines instead? The previous version had a 
number of spaces as well, but since `define' is not creating additional indent 
like `let' should, I leave those away.

Thank you for the feedback!

I have a question or suggestion:

When I save the file in Emacs, my Emacs turns all the tabs in there into spaces. 
Probably my own custom global config's choice about indentation. Could a general 
mode line thing be added to avoid that and nail down the current formatting 
style, so that contributors only need to allow Emacs to run those settings and 
then not need to care about it? Currently the indentation style seems to be a 
mix of tabs and spaces.

(For example the GNU Guix project does a lot of formatting things in their 
files, that configure Emacs, so that the formatting will be automatically 
according to their practices.)

Otherwise the diff becomes huge and I have to discard all those tab -> spaces 
changes again, before I can make a patch. Or I have to edit in fundamental mode, 
which loses syntax highlighting and is what I have been doing. But even in 
fundamental-mode Emacs changes some of the indentation from tabs to spaces or 
spaces to tabs apparently. Maybe I should be editing with emacs -Q or something? 
But then I don't have magit. This creates quite some friction in the editing 
process for me. In fundamental mode, I have to discard 4-6 chunks, which I did 
not even touch, but which were changed in terms of what the indentation consists of.

And one more question:

Does the name of the patch file matter? Git changed the name and I will attach 
it as it was created by git. Hope that's alright.

Attached: The updated patch.

And of course: Let me know if it needs more changes.

Regards,
Zelphir

-- 
repositories:https://notabug.org/ZelphirKaltstahl

[-- Attachment #1.2: Type: text/html, Size: 8431 bytes --]

[-- Attachment #2: 0001-org-babel-expand-body-scheme-define-header-arg-vars-.patch --]
[-- Type: text/x-patch, Size: 2263 bytes --]

From 99a38e67a16c7541ce2d5da2d8d46e5914e559ae Mon Sep 17 00:00:00 2001
From: Zelphir Kaltstahl <zelphirkaltstahl@posteo.de>
Date: Sat, 25 Mar 2023 15:21:38 +0100
Subject: [PATCH] org-babel-expand-body:scheme: define header arg vars using
 define

* ob-scheme.el (org-babel-expand-body:scheme,
org-babel-expand-header-arg-vars:scheme): Change the way header
argument :var variables are expanded for for Scheme source blocks.  Use
`define' instead of wrapping using `let'.

Wrapping binding definitions using `let' can lead to issues with GNU
Guile and potentially other Scheme dialects.  GNU Guile will only get
to the body of the let at evaluation time, not at macro expansion
time.  If the let form wraps any imports of libraries that define
macros, then those imported macros are seen too late and their
corresponding forms inside the body of the let are not
expanded.  Using `define' to define bindings avoids this problem, at
least in GNU Guile.

For more context see the mailing list discussion at: https://lists.gnu.org/archive/html/emacs-orgmode/2023-03/msg00087.html
---
 lisp/ob-scheme.el | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/lisp/ob-scheme.el b/lisp/ob-scheme.el
index 9f12f42cb..7424ab652 100644
--- a/lisp/ob-scheme.el
+++ b/lisp/ob-scheme.el
@@ -79,6 +79,14 @@
 (defvar org-babel-default-header-args:scheme '()
   "Default header arguments for scheme code blocks.")
 
+(defun org-babel-scheme-expand-header-arg-vars (vars)
+  "Expand :var header arguments given as VARS."
+  (mapconcat
+   (lambda (var)
+     (format "(define %S %S)" (car var) (cdr var)))
+   vars
+   "\n"))
+
 (defun org-babel-expand-body:scheme (body params)
   "Expand BODY according to PARAMS, return the expanded body."
   (let ((vars (org-babel--get-vars params))
@@ -86,13 +94,7 @@
 	(postpends (cdr (assq :epilogue params))))
     (concat (and prepends (concat prepends "\n"))
 	    (if (null vars) body
-	      (format "(let (%s)\n%s\n)"
-		      (mapconcat
-		       (lambda (var)
-			 (format "%S" (print `(,(car var) ',(cdr var)))))
-		       vars
-		       "\n      ")
-		      body))
+	      (concat (org-babel-scheme-expand-header-arg-vars vars) body))
 	    (and postpends (concat "\n" postpends)))))
 
 
-- 
2.25.1

Re: [PATCH] lisp/ob-scheme.el

[Ihor Radchenko]

Zelphir Kaltstahl <zelphirkaltstahl@posteo.de> writes:

>>> +  "Expand :var header arguments given as VARS."
>>> +  (mapconcat
>>> +   (lambda (var)
>>> +     (format "(define %s %S)" (car var) (cdr var)))
>> Is there any reason why you use %s for variable name? Previously it was
>> formatted with escapes (using %S).
>
> That was me thinking: "The name of the variable should just be itself, not 
> wrapped in double quotes, because in Scheme I cannot create a variable as 
> (define "abc" 123)". But maybe I misunderstood %s and %S. I also do not know, 
> how elisp's `print' treats its arguments. Will use 2 times %S then.

The main difference between %S and %s can be demonstrated using the
following:

(insert (format "%s" 'symbol\#w\ spaces))
(insert (format "%S" 'symbol\#w\ spaces))

Basically, if symbol name contains funny chars, reader may need to have
them escaped. %S does the escaping, while %s does not.

Of course, %S escapes according to Elisp reader, not Scheme.
I am not sure if Scheme syntax allows escapes in symbol names.
If not, there should be no real difference.

Also, `print' is the same as %S.

>> Also, previous version quoted the variable value with "'". Why didn't
>> you do it here?
>
> I am not sure I understand what you are referring to in the previous version. Do 
> you mean that `print' quoted variable values with a single quote? Do you mean 
> this part of the previous code:
>
> (print `(,(car var) ',(cdr var)))
>
> ?

Yes, but upon closer look, it should not matter. Your code will work
just fine.

>>> +	      (concat (org-babel-expand-header-arg-vars:scheme vars) body))
>> `mapconcat' you used in `org-babel-expand-header-arg-vars:scheme' does
>> not add trailing newline, unlike done previously.

> Am I not adding a newline? I think I do?:

I meant after variable definitions.
You have
   (concat (org-babel-scheme-expand-header-arg-vars vars) body)
but string returned by `org-babel-scheme-expand-header-arg-vars' will
not have final newline after the very last define.

> I have a question or suggestion:
>
> When I save the file in Emacs, my Emacs turns all the tabs in there into spaces. 
> Probably my own custom global config's choice about indentation. Could a general 
> mode line thing be added to avoid that and nail down the current formatting 
> style, so that contributors only need to allow Emacs to run those settings and 
> then not need to care about it? Currently the indentation style seems to be a 
> mix of tabs and spaces.

This is something about your setup overriding Org mode project defaults.
If you open org-mode/.dir-locals.el, you will see that we enable
indent-tabs-mode. If you still do not get it enabled, you probably
set indentation defaults aggressively it in your config via hooks or
something similar.

> And one more question:
>
> Does the name of the patch file matter? Git changed the name and I will attach 
> it as it was created by git. Hope that's alright.

Patch file name does not matter.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
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>

Re: [PATCH] lisp/ob-scheme.el

[Ihor Radchenko]

Zelphir Kaltstahl <zelphirkaltstahl@posteo.de> writes:

> Subject: [PATCH] org-babel-expand-body:scheme: define header arg vars using
>  define

Applied, onto main, adding TINYCHANGE cookie and newline between
variable definitions and body.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=d97ba5ba5

You are also now listed as Org contributor, thanks!
https://git.sr.ht/~bzg/worg/commit/eee4cb25

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
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>

Fixed.

Re: [PATCH] lisp/ob-scheme.el

[Zelphir Kaltstahl]

On 4/25/23 14:28, Ihor Radchenko wrote:
> Zelphir Kaltstahl <zelphirkaltstahl@posteo.de> writes:
>
>> Subject: [PATCH] org-babel-expand-body:scheme: define header arg vars using
>>   define
> Applied, onto main, adding TINYCHANGE cookie and newline between
> variable definitions and body.
> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=d97ba5ba5
>
> You are also now listed as Org contributor, thanks!
> https://git.sr.ht/~bzg/worg/commit/eee4cb25

Thank you for taking care of the remaining work!

Best wishes,
Zelphir

-- 
repositories: https://notabug.org/ZelphirKaltstahl

Re: org-babel guile source block bug in handling multiple values

[Ihor Radchenko]

Ihor Radchenko <yantar92@posteo.net> writes:

> Zelphir Kaltstahl <zelphirkaltstahl@posteo.de> writes:
>
>>> This is expected. Noweb includes the src block code without altering it.
>>> See 16.11 Noweb Reference Syntax
>>>
>>> We may probably clarify this in the manual. Would it be helpful?
>> I think it would be helpful. I merely tried Python, because it was at hand and 
>> because I did not find ob-racket in M-x package-list RET and did not have 
>> another Scheme installed. Cannot say, whether others find the behavior confusing 
>> or not, but I imagine I would find it helpful, if it was mentioned in the docs.
>
> See the attached tentative patch.

Applied, with amendments. I moved the example to a footnote. Otherwise,
the section flow does not read right.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=dbb451dc9

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
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>