[PATCH] Fix behavior of lambda default header arg vars

[PATCH] Fix behavior of lambda default header arg vars

[Matt Huszagh]

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

Hello,

:var header arguments can be provided multiple times. This is supported
directly at the source block and through the default header argument
facility. However, this was not handled correctly when the var was
evaluated from a closure in a default header argument (only the last var
was taken). This patch fixes that. I've also added documentation
explaining how to set multiple vars in the default header argument
facility.

Thanks,
Matt


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-ob-core.el-Fix-behavior-of-lambda-default-header-arg.patch --]
[-- Type: text/x-patch, Size: 2668 bytes --]

From 5f02fe4e896e6909fab18e8d90a7e14bdce9d5c4 Mon Sep 17 00:00:00 2001
From: Matt Huszagh <huszaghmatt@gmail.com>
Date: Wed, 27 Oct 2021 07:47:20 -0700
Subject: [PATCH] ob-core.el: Fix behavior of lambda default header arg vars

* lisp/ob-core.el: Permit multiple :var default header arguments when
using closures. Additionally, document how to use default header
arguments for arguments that can be provided multiple times.
---
 lisp/ob-core.el | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/lisp/ob-core.el b/lisp/ob-core.el
index 7a9467b0e..21f461a08 100644
--- a/lisp/ob-core.el
+++ b/lisp/ob-core.el
@@ -478,12 +478,21 @@ For the format of SAFE-LIST, see `org-babel-safe-header-args'."
 This is a list in which each element is an alist.  Each key
 corresponds to a header argument, and each value to that header's
 value.  The value can either be a string or a closure that
-evaluates to a string.  The closure is evaluated when the source
-block is being evaluated (e.g. during execution or export), with
-point at the source block.  It is not possible to use an
-arbitrary function symbol (e.g. 'some-func), since org uses
-lexical binding.  To achieve the same functionality, call the
-function within a closure (e.g. (lambda () (some-func))).
+evaluates to a string.  Some header arguments (e.g., :var for
+some language backends) can be provided multiple times for a
+source block.  This functionality is also supported for default
+header arguments by providing the header argument multiple times
+in the alist.  For example:
+
+'((:var . \"foo=\\\"bar\\\"\")
+  (:var . \"bar=\\\"foo\\\"\"))
+
+A closure is evaluated when the source block is being
+evaluated (e.g. during execution or export), with point at the
+source block.  It is not possible to use an arbitrary function
+symbol (e.g. 'some-func), since org uses lexical binding.  To
+achieve the same functionality, call the function within a
+closure (e.g. (lambda () (some-func))).
 
 To understand how closures can be used as default header
 arguments, imagine you'd like to set the file name output of a
@@ -2718,6 +2727,11 @@ parameters when merging lists."
 	(pcase pair
 	  (`(:var . ,value)
 	   (let ((name (cond
+                        ;; Default header arguments can accept lambda
+                        ;; functions. We uniquely identify the var
+                        ;; according to the full string contents of
+                        ;; the lambda function.
+			((functionp value) value)
 			((listp value) (car value))
 			((string-match "^\\([^= \f\t\n\r\v]+\\)[ \t]*=" value)
 			 (intern (match-string 1 value)))
-- 
2.31.1

Re: [PATCH] Fix behavior of lambda default header arg vars

[Matt Huszagh]

Matt Huszagh <huszaghmatt@gmail.com> writes:

> Hey Timothy,
>
> Any thoughts on this bug fix?
>
> Thanks
> Matt

Apologies, meant to include the org-mode mailing list in this too.

Thanks
Matt

Re: [PATCH] Fix behavior of lambda default header arg vars

[Ihor Radchenko]

Matt Huszagh <huszaghmatt@gmail.com> writes:

> :var header arguments can be provided multiple times. This is supported
> directly at the source block and through the default header argument
> facility. However, this was not handled correctly when the var was
> evaluated from a closure in a default header argument (only the last var
> was taken). This patch fixes that. I've also added documentation
> explaining how to set multiple vars in the default header argument
> facility.

Thanks for the patch and sorry for the late reply.

> -arbitrary function symbol (e.g. 'some-func), since org uses
> -lexical binding.  To achieve the same functionality, call the
> -function within a closure (e.g. (lambda () (some-func))).
> +evaluates to a string.  Some header arguments (e.g., :var for
> +some language backends) can be provided multiple times for a
> +source block.  This functionality is also supported for default
> +header arguments by providing the header argument multiple times
> +in the alist.  For example:
> +
> +'((:var . \"foo=\\\"bar\\\"\")
> +  (:var . \"bar=\\\"foo\\\"\"))
> +
> +A closure is evaluated when the source block is being
> +evaluated (e.g. during execution or export), with point at the
> +source block.  It is not possible to use an arbitrary function
> +symbol (e.g. 'some-func), since org uses lexical binding.  To
> +achieve the same functionality, call the function within a
> +closure (e.g. (lambda () (some-func))).

The new docstring is confusing. The same paragraph is talking about
closures, then multiple header args, and gives an example without
closures.

It would help if closure part and multi-variable part were split into
separate paragraphs. Also, the example is not helpful here. Are you
saying that _only some_ backends support multiple vars? Are there
backends that do not support? Or are you talking about multiple
assignments to the same variable?

Best,
Ihor

Re: [PATCH] Fix behavior of lambda default header arg vars

[Matt Huszagh]

> It would help if closure part and multi-variable part were split into
> separate paragraphs.

The closure part and muliple header argument part are already in
separate paragraphs. The multiple header argument part, however, is
incorporated into an introductory paragraph that very briefly describes
the value syntax of org-babel-default-header-args and the types of alist
values it supports. I did this because that introductory information is
short and simple and so I felt it acceptable to incorporate the multiple
header argument information. I don't feel this places a significant
intellectual burden on the reader to follow, but I'm happy to insert a
newline before "Some header arguments..." if you'd prefer.

> Are you saying that _only some_ backends support multiple vars? Are
> there backends that do not support?

I think you're confused about "(e.g., :var for some language backends)":

It's been a while since I created this patch and I don't remember
exactly why I wrote it this way. I think it was based on the belief that
not all language backends support variable passing in header arguments,
though I honestly couldn't tell you at the moment whether this is
true. In that vein, a semantically equivalent way to write this would be
"(e.g., :var for language backends that support it)". Maybe all language
backends support variable header arguments, in which case "(e.g., :var)"
could be used here instead. In any case, the "some language backends"
part of the phrase is not a qualification of "multiple", but of
"variables". Nor is it correct to read it this way, as the statement is
grammatically clear and correct. An equivalent statement would be:

"""
Some header arguments can be provided multiple times for a source
block. An example of such a header argument is :var.
"""

> Or are you talking about multiple assignments to the same variable?

No. The topic of multiple assignments to the same variable is not
discussed here.

> Also, the example is not helpful here.

The example *is* helpful. It provides explicit direction for how to
handle the non-obvious case of header arguments that can be passed
multiple times, which isn't described much in the documentation.

> The new docstring is confusing. The same paragraph is talking about
> closures, then multiple header args, and gives an example without
> closures.

I don't think there's anything particularly confusing about this
docstring. As already mentioned, that paragraph discusses acceptable
alist values and multiple header args. Closures are mentioned only as
one of the acceptable alist values, which puts them on the same level as
strings. The more in-depth discussion of closures happens later, in its
own paragraphs.

Taking a step back, the structure of this docstring is:

1. A single sentence briefly describing org-babel-default-header-args.
2. A description of the syntax and acceptable values that can be
   assigned to org-babel-default-header-args.
3. A discussion of how to handle header arguments that can be provided
   multiple times. This explicit treatment is justified because this
   case is not intuitively obvious.
4. A discussion of closures (including why functions need to be provided
   as closures) and an example illustrating their usefulness.

2 and 3 are part of the same paragraph and 4 is several paragraphs (the
precise number depends on whether you consider code blocks to be their
own paragraph and how they break up surrounding text). This is a
perfectly reasonable way to structure this information. There's no rule
requiring that there be one topic per paragraph. Instead, paragraphs
should be structured in a way that's clear, and in many cases (not all)
that does result in a paragraph discussing one topic. But again, a
reasonable alternative structure would be to separate 2 and 3 into their
own paragraphs and I'm happy to do that. The multiple header argument
discussion also doesn't need to be located where it is. It could
alternatively go after the discussion of closures if you prefer that.

Matt

Re: [PATCH] Fix behavior of lambda default header arg vars

[Ihor Radchenko]

Matt Huszagh <huszaghmatt@gmail.com> writes:

>> It would help if closure part and multi-variable part were split into
>> separate paragraphs.
>
> The closure part and muliple header argument part are already in
> separate paragraphs. The multiple header argument part, however, is
> incorporated into an introductory paragraph that very briefly describes
> the value syntax of org-babel-default-header-args and the types of alist
> values it supports. I did this because that introductory information is
> short and simple and so I felt it acceptable to incorporate the multiple
> header argument information. I don't feel this places a significant
> intellectual burden on the reader to follow, but I'm happy to insert a
> newline before "Some header arguments..." if you'd prefer.

Yes, please. And also move the multiple header arg parts below the
closure example.

>> Are you saying that _only some_ backends support multiple vars? Are
>> there backends that do not support?
>
> I think you're confused about "(e.g., :var for some language backends)":
>
> It's been a while since I created this patch and I don't remember
> exactly why I wrote it this way. I think it was based on the belief that
> not all language backends support variable passing in header arguments,
> though I honestly couldn't tell you at the moment whether this is
> true. In that vein, a semantically equivalent way to write this would be
> "(e.g., :var for language backends that support it)". Maybe all language
> backends support variable header arguments, in which case "(e.g., :var)"
> could be used here instead. In any case, the "some language backends"
> part of the phrase is not a qualification of "multiple", but of
> "variables". Nor is it correct to read it this way, as the statement is
> grammatically clear and correct. An equivalent statement would be:
>
> """
> Some header arguments can be provided multiple times for a source
> block. An example of such a header argument is :var.
> """

This looks slightly better. Though after reading your reply and
reviewing the docstring, I realize that my confusion partially came from
the fact that your patch is about closures, while the docstring change
is not.

Could you split that patch in two parts: one for the docstring change
and one for the :var parameter handling?

>> Also, the example is not helpful here.
>
> The example *is* helpful. It provides explicit direction for how to
> handle the non-obvious case of header arguments that can be passed
> multiple times, which isn't described much in the documentation.

Agree. See my above explanation about the source of confusion.

Best,
Ihor

Re: [PATCH] Fix behavior of lambda default header arg vars

[Matt Huszagh]

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

Hi Ihor,

Modifications made and split into two patches. Let me know if any
remaining issues.

Thanks
Matt


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-ob-core.el-Fix-behavior-of-lambda-default-header-arg.patch --]
[-- Type: text/x-patch, Size: 1052 bytes --]

From fdc6e103932c68b450c72d44d062b4746ec2202c Mon Sep 17 00:00:00 2001
From: Matt Huszagh <huszaghmatt@gmail.com>
Date: Mon, 27 Jun 2022 20:41:02 -0700
Subject: [PATCH 1/2] ob-core.el: Fix behavior of lambda default header arg
 vars

* lisp/ob-core.el: Permit multiple :var default header arguments when
using closures.
---
 lisp/ob-core.el | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/lisp/ob-core.el b/lisp/ob-core.el
index 7a9467b0e..80086f12c 100644
--- a/lisp/ob-core.el
+++ b/lisp/ob-core.el
@@ -2718,6 +2718,11 @@ parameters when merging lists."
 	(pcase pair
 	  (`(:var . ,value)
 	   (let ((name (cond
+                        ;; Default header arguments can accept lambda
+                        ;; functions. We uniquely identify the var
+                        ;; according to the full string contents of
+                        ;; the lambda function.
+			((functionp value) value)
 			((listp value) (car value))
 			((string-match "^\\([^= \f\t\n\r\v]+\\)[ \t]*=" value)
 			 (intern (match-string 1 value)))
-- 
2.31.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-ob-core.el-Improve-org-babel-default-header-args-doc.patch --]
[-- Type: text/x-patch, Size: 2553 bytes --]

From 7af7ad3990410ca388c841e045ec66ce694b404a Mon Sep 17 00:00:00 2001
From: Matt Huszagh <huszaghmatt@gmail.com>
Date: Mon, 27 Jun 2022 20:42:27 -0700
Subject: [PATCH 2/2] ob-core.el: Improve org-babel-default-header-args
 docstring

* lisp/ob-core.el: Provide example illustrating one benefit of using
closures as default header arguments. Additionally, explain how to
provide the same type of header argument multiple times in the default
alist.
---
 lisp/ob-core.el | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/lisp/ob-core.el b/lisp/ob-core.el
index 80086f12c..6c3c79126 100644
--- a/lisp/ob-core.el
+++ b/lisp/ob-core.el
@@ -478,12 +478,14 @@ For the format of SAFE-LIST, see `org-babel-safe-header-args'."
 This is a list in which each element is an alist.  Each key
 corresponds to a header argument, and each value to that header's
 value.  The value can either be a string or a closure that
-evaluates to a string.  The closure is evaluated when the source
-block is being evaluated (e.g. during execution or export), with
-point at the source block.  It is not possible to use an
-arbitrary function symbol (e.g. 'some-func), since org uses
-lexical binding.  To achieve the same functionality, call the
-function within a closure (e.g. (lambda () (some-func))).
+evaluates to a string.
+
+A closure is evaluated when the source block is being
+evaluated (e.g. during execution or export), with point at the
+source block.  It is not possible to use an arbitrary function
+symbol (e.g. 'some-func), since org uses lexical binding.  To
+achieve the same functionality, call the function within a
+closure (e.g. (lambda () (some-func))).
 
 To understand how closures can be used as default header
 arguments, imagine you'd like to set the file name output of a
@@ -500,7 +502,16 @@ this with:
 
 Because the closure is evaluated with point at the source block,
 the call to `org-element-at-point' above will always retrieve
-information about the current source block.")
+information about the current source block.
+
+Some header arguments can be provided multiple times for a source
+block.  An example of such a header argument is :var.  This
+functionality is also supported for default header arguments by
+providing the header argument multiple times in the alist.  For
+example:
+
+'((:var . \"foo=\\\"bar\\\"\")
+  (:var . \"bar=\\\"foo\\\"\"))")
 
 (put 'org-babel-default-header-args 'safe-local-variable
      (org-babel-header-args-safe-fn org-babel-safe-header-args))
-- 
2.31.1

Re: [PATCH] Fix behavior of lambda default header arg vars

[Ihor Radchenko]

Matt Huszagh <huszaghmatt@gmail.com> writes:

> Modifications made and split into two patches. Let me know if any
> remaining issues.

> From 7af7ad3990410ca388c841e045ec66ce694b404a Mon Sep 17 00:00:00 2001
> From: Matt Huszagh <huszaghmatt@gmail.com>
> Date: Mon, 27 Jun 2022 20:42:27 -0700
> Subject: [PATCH 2/2] ob-core.el: Improve org-babel-default-header-args
>  docstring

This patch does not apply. It does not appear to be based on the
original main branch:

Applying: ob-core.el: Improve org-babel-default-header-args docstring
error: sha1 information is lacking or useless (lisp/ob-core.el).
error: could not build fake ancestor

Best,
Ihor

Re: [PATCH] Fix behavior of lambda default header arg vars

[Matt Huszagh]

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

Ihor Radchenko <yantar92@gmail.com> writes:

> This patch does not apply. It does not appear to be based on the
> original main branch:
>
> Applying: ob-core.el: Improve org-babel-default-header-args docstring
> error: sha1 information is lacking or useless (lisp/ob-core.el).
> error: could not build fake ancestor

My mistake. Try these.

Matt


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-ob-core.el-Fix-behavior-of-lambda-default-header-arg.patch --]
[-- Type: text/x-patch, Size: 1052 bytes --]

From 6894d7b441fe975bc90177d85102c031cffe3c46 Mon Sep 17 00:00:00 2001
From: Matt Huszagh <huszaghmatt@gmail.com>
Date: Mon, 27 Jun 2022 20:41:02 -0700
Subject: [PATCH 1/2] ob-core.el: Fix behavior of lambda default header arg
 vars

* lisp/ob-core.el: Permit multiple :var default header arguments when
using closures.
---
 lisp/ob-core.el | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/lisp/ob-core.el b/lisp/ob-core.el
index 75a6a167d..0d9ec5c84 100644
--- a/lisp/ob-core.el
+++ b/lisp/ob-core.el
@@ -2749,6 +2749,11 @@ parameters when merging lists."
 	(pcase pair
 	  (`(:var . ,value)
 	   (let ((name (cond
+                        ;; Default header arguments can accept lambda
+                        ;; functions. We uniquely identify the var
+                        ;; according to the full string contents of
+                        ;; the lambda function.
+			((functionp value) value)
 			((listp value) (car value))
 			((string-match "^\\([^= \f\t\n\r\v]+\\)[ \t]*=" value)
 			 (intern (match-string 1 value)))
-- 
2.31.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-ob-core.el-Improve-org-babel-default-header-args-doc.patch --]
[-- Type: text/x-patch, Size: 2556 bytes --]

From 12833bc6c089af9d94c9535835e0a68d189c8496 Mon Sep 17 00:00:00 2001
From: Matt Huszagh <huszaghmatt@gmail.com>
Date: Mon, 27 Jun 2022 20:42:27 -0700
Subject: [PATCH 2/2] ob-core.el: Improve org-babel-default-header-args
 docstring

* lisp/ob-core.el: Provide example illustrating one benefit of using
closures as default header arguments. Additionally, explain how to
provide the same type of header argument multiple times in the default
alist.
---
 lisp/ob-core.el | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/lisp/ob-core.el b/lisp/ob-core.el
index 0d9ec5c84..d85c66e76 100644
--- a/lisp/ob-core.el
+++ b/lisp/ob-core.el
@@ -482,12 +482,14 @@ For the format of SAFE-LIST, see `org-babel-safe-header-args'."
 This is a list in which each element is an alist.  Each key
 corresponds to a header argument, and each value to that header's
 value.  The value can either be a string or a closure that
-evaluates to a string.  The closure is evaluated when the source
-block is being evaluated (e.g. during execution or export), with
-point at the source block.  It is not possible to use an
-arbitrary function symbol (e.g. \\='some-func), since org uses
-lexical binding.  To achieve the same functionality, call the
-function within a closure (e.g. (lambda () (some-func))).
+evaluates to a string.
+
+A closure is evaluated when the source block is being
+evaluated (e.g. during execution or export), with point at the
+source block.  It is not possible to use an arbitrary function
+symbol (e.g. 'some-func), since org uses lexical binding.  To
+achieve the same functionality, call the function within a
+closure (e.g. (lambda () (some-func))).
 
 To understand how closures can be used as default header
 arguments, imagine you'd like to set the file name output of a
@@ -504,7 +506,16 @@ this with:
 
 Because the closure is evaluated with point at the source block,
 the call to `org-element-at-point' above will always retrieve
-information about the current source block.")
+information about the current source block.
+
+Some header arguments can be provided multiple times for a source
+block.  An example of such a header argument is :var.  This
+functionality is also supported for default header arguments by
+providing the header argument multiple times in the alist.  For
+example:
+
+'((:var . \"foo=\\\"bar\\\"\")
+  (:var . \"bar=\\\"foo\\\"\"))")
 
 (put 'org-babel-default-header-args 'safe-local-variable
      (org-babel-header-args-safe-fn org-babel-safe-header-args))
-- 
2.31.1

Re: [PATCH] Fix behavior of lambda default header arg vars

[Ihor Radchenko]

Matt Huszagh <huszaghmatt@gmail.com> writes:

> ... Try these.

Thanks!
Applied onto main via e8dd98d15 and 8be689f11 with amendments.
I have modified the commit messages to provide the properly formatted
changelog entries that include the modified function/defcustom name.
Also, I have changed the commit messages and the comments to have double
space between sentences.

Best,
Ihor