Re: [FR] :noweb-wrap header arg

[Ihor Radchenko]

Amy Grinn  writes:

>> I recommend the following:
>>
>> If the value starts from ", use Elisp's `read':
>>|"# <<" ">>"
>>(read (current-buffer)) ; => "# <<"
>>otherwise, consider read until the first whitespace.
>>|#<<; >>;
>>(re-search-forward (rx (1+ (not whitespace
>>#<<;|
>>
>> However, there may be edge cases like
>>
>> "<< >>"
>> "<< >>
>> << << >>
>> << "asd" >>
>
> I didn't implement this recommendation yet; I still think the regex is a
> more clear way of putting it, even without using a temporary buffer:
>
> ;; If a pair of " is found separated by one or more
> ;; characters, capture those characters as a group
> (unless (eq i (string-match (rx (* space) ?\"
> (group (+ (not ?\")))
> ?\" (* space))
> raw i))
>   ;; Otherwise, capture the next non-whitespace group of
>   ;; characters
>   (string-match (rx (* space) (group (* (not space))) (* space))
> raw i))
>
> Let me know what you think!

Consider cases like

"< \"" "\" >"

or

"< >"

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 

Re: [FR] :noweb-wrap header arg

[Amy Grinn]

Ihor Radchenko  writes:

> Amy Grinn  writes:
>
>>> +1
>>> You may even use obsolete alias (add it to lisp/org-compat.el)
>>
>> Here's a patch to rename org-babel-noweb-wrap to
>> org-babel-noweb-make-regexp.
>
> Thanks!
> News entry is not necessary here - we are just renaming a function.
> Otherwise, the patch looks good.
> I am not yet merging it though - let's have all the patches you plan for
> the new noweb-wrap argument first and then apply them together.

Got it, not a problem!  I've attached both patches here.

Regarding the other conversation,

 +(while (< (point) (point-max))
 +  (unless (looking-at " *\"\\([^\"]+\\)\" *")
 +(looking-at " *\\([^ ]+\\)"))
>>>
>>> May you please explain the rationale behind this regexp? AFAIU, it
>>> implies that you want to allow whitespace characters inside :noweb-wrap
>>> boundaries. But I do not think that we need to complicate things so much.
>>
>> That is exactly what I was going for.  I thought about the ways this
>> could be used and the most general-purpose, non-syntax-breaking,
>> easily-recognizable way I could think of was to use the language's
>> line-comment construct followed by the standard << >> characters.
>>
>> # <>
>> ;; <>
>> // <>
>>
>> I can see how it might be harder to maintain to allow whitespace in the
>> noweb-wrap header arg.  I could create a separate org-parse-arg-list
>> function to ease that burden somewhat.  My opinion is that having the
>> option to use the examples above is preferable to using non-standard
>> wrappers, from a third-person point-of-view.
>
> Makes sense. Also, see a similar idea being discussed in
> https://list.orgmode.org/orgmode/87o7jlzxgn.fsf@localhost/
>
> I recommend the following:
>
> If the value starts from ", use Elisp's `read':
>|"# <<" ">>"
>(read (current-buffer)) ; => "# <<"
>otherwise, consider read until the first whitespace.
>|#<<; >>;
>(re-search-forward (rx (1+ (not whitespace
>#<<;|
>
> However, there may be edge cases like
>
> "<< >>"
> "<< >>
> << << >>
> << "asd" >>

I didn't implement this recommendation yet; I still think the regex is a
more clear way of putting it, even without using a temporary buffer:

;; If a pair of " is found separated by one or more
;; characters, capture those characters as a group
(unless (eq i (string-match (rx (* space) ?\"
(group (+ (not ?\")))
?\" (* space))
raw i))
  ;; Otherwise, capture the next non-whitespace group of
  ;; characters
  (string-match (rx (* space) (group (* (not space))) (* space))
raw i))

Let me know what you think!
>From 9442c029a7b2f1ec061e42a047b3d1bff88441d8 Mon Sep 17 00:00:00 2001
From: Amy Grinn 
Date: Wed, 17 Apr 2024 16:01:40 -0400
Subject: [PATCH 1/2] lisp/ob-core.el: (org-babel-noweb-wrap): renamed to
 org-babel-noweb-make-regexp

* lisp/org-compat.el: Declare org-babel-noweb-wrap to be an obselete
function alias for org-babel-noweb-make-regexp.
* lisp/ob-core.el (org-babel-noweb-make-regexp): Rename the function.
(org-babel-goto-named-src-block):
(org-babel-expand-noweb-references):
* lisp/ob-exp.el (org-babel-exp-code):
* lisp/ob-tangle.el (org-babel-tangle-clean):
(org-babel-tangle-single-block): Use the new function name.
---
 lisp/ob-core.el| 8 
 lisp/ob-exp.el | 2 +-
 lisp/ob-tangle.el  | 5 +++--
 lisp/org-compat.el | 2 +-
 4 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/lisp/ob-core.el b/lisp/ob-core.el
index c5dd20b0e..1518d7726 100644
--- a/lisp/ob-core.el
+++ b/lisp/ob-core.el
@@ -194,7 +194,7 @@ This string must include a \"%s\" which will be replaced by the results."
   :package-version '(Org . "9.1")
   :safe #'booleanp)
 
-(defun org-babel-noweb-wrap ( regexp)
+(defun org-babel-noweb-make-regexp ( regexp)
   "Return regexp matching a Noweb reference.
 
 Match any reference, or only those matching REGEXP, if non-nil.
@@ -1976,7 +1976,7 @@ src block, then return nil."
 		   (type (org-element-type context))
 		   (noweb-ref
 		(and (memq type '(inline-src-block src-block))
-			 (org-in-regexp (org-babel-noweb-wrap)
+			 (org-in-regexp (org-babel-noweb-make-regexp)
 	  (cond
 	   (noweb-ref
 		(buffer-substring
@@ -3125,7 +3125,7 @@ block but are passed literally to the \"example-block\"."
   (not (equal (cdr v) "no"))
 	 (noweb-re (format "\\(.*?\\)\\(%s\\)"
 			   (with-current-buffer parent-buffer
-			 (org-babel-noweb-wrap)
+			 (org-babel-noweb-make-regexp)
 (unless (equal (cons parent-buffer
  (with-current-buffer parent-buffer
(buffer-chars-modified-tick)))
@@ -3175,7 +3175,7 @@ block but are passed literally to the \"example-block\"."
 	   ((guard (or org-babel-noweb-error-all-langs
 			   (member lang 

Re: [FR] :noweb-wrap header arg

[Ihor Radchenko]

Amy Grinn  writes:

>> +1
>> You may even use obsolete alias (add it to lisp/org-compat.el)
>
> Here's a patch to rename org-babel-noweb-wrap to
> org-babel-noweb-make-regexp.

Thanks!
News entry is not necessary here - we are just renaming a function.
Otherwise, the patch looks good.
I am not yet merging it though - let's have all the patches you plan for
the new noweb-wrap argument first and then apply them together.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 

Re: [FR] :noweb-wrap header arg

[Amy Grinn]

Ihor Radchenko  writes:

> Amy Grinn  writes:
>
>> First of all, I would like to change (defalias) the function name
>> org-babel-noweb-wrap to org-babel-noweb-make-regexp. I think this in
>> more in line with other functions which create regular expressions.
>
> +1
> You may even use obsolete alias (add it to lisp/org-compat.el)

Here's a patch to rename org-babel-noweb-wrap to
org-babel-noweb-make-regexp.

>From b318fef6af8ae47b7e6d0371ccc87a01ed1a7755 Mon Sep 17 00:00:00 2001
From: Amy Grinn 
Date: Wed, 17 Apr 2024 16:01:40 -0400
Subject: [PATCH] lisp/ob-core.el: (org-babel-noweb-wrap): renamed to
 org-babel-noweb-make-regexp

* etc/ORG-NEWS (~org-babel-noweb-wrap~ is now
~org-babel-noweb-make-regexp~): Announce the change.
* lisp/org-compat.el: Declare org-babel-noweb-wrap to be an obselete
function alias for org-babel-noweb-make-regexp.
* lisp/ob-core.el (org-babel-noweb-make-regexp): Rename the function.
(org-babel-goto-named-src-block):
(org-babel-expand-noweb-references):
* lisp/ob-exp.el (org-babel-exp-code):
* lisp/ob-tangle.el (org-babel-tangle-clean):
(org-babel-tangle-single-block): Use the new function name.
---
 etc/ORG-NEWS   | 6 ++
 lisp/ob-core.el| 8 
 lisp/ob-exp.el | 2 +-
 lisp/ob-tangle.el  | 5 +++--
 lisp/org-compat.el | 2 +-
 5 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index 978882a7a..97e2f2e3f 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -1563,6 +1563,12 @@ optional argument =NEW-HEADING-CONTAINER= specifies where in the
 buffer it will be added.  If not specified, new headings are created
 at level 1 at the end of the accessible part of the buffer, as before.
 
+** Removed or renamed functions and variables
+*** ~org-babel-noweb-wrap~ is now ~org-babel-noweb-make-regexp~
+
+This is more in line with other functions that return a regular
+expression.
+
 ** Miscellaneous
 *** =org-crypt.el= now applies initial visibility settings to decrypted entries
 
diff --git a/lisp/ob-core.el b/lisp/ob-core.el
index c5dd20b0e..1518d7726 100644
--- a/lisp/ob-core.el
+++ b/lisp/ob-core.el
@@ -194,7 +194,7 @@ This string must include a \"%s\" which will be replaced by the results."
   :package-version '(Org . "9.1")
   :safe #'booleanp)
 
-(defun org-babel-noweb-wrap ( regexp)
+(defun org-babel-noweb-make-regexp ( regexp)
   "Return regexp matching a Noweb reference.
 
 Match any reference, or only those matching REGEXP, if non-nil.
@@ -1976,7 +1976,7 @@ src block, then return nil."
 		   (type (org-element-type context))
 		   (noweb-ref
 		(and (memq type '(inline-src-block src-block))
-			 (org-in-regexp (org-babel-noweb-wrap)
+			 (org-in-regexp (org-babel-noweb-make-regexp)
 	  (cond
 	   (noweb-ref
 		(buffer-substring
@@ -3125,7 +3125,7 @@ block but are passed literally to the \"example-block\"."
   (not (equal (cdr v) "no"))
 	 (noweb-re (format "\\(.*?\\)\\(%s\\)"
 			   (with-current-buffer parent-buffer
-			 (org-babel-noweb-wrap)
+			 (org-babel-noweb-make-regexp)
 (unless (equal (cons parent-buffer
  (with-current-buffer parent-buffer
(buffer-chars-modified-tick)))
@@ -3175,7 +3175,7 @@ block but are passed literally to the \"example-block\"."
 	   ((guard (or org-babel-noweb-error-all-langs
 			   (member lang org-babel-noweb-error-langs)))
 	(error "Cannot resolve %s (see `org-babel-noweb-error-langs')"
-		   (org-babel-noweb-wrap ,ref)))
+		   (org-babel-noweb-make-regexp ,ref)))
 	   (_ ""
   (replace-regexp-in-string
noweb-re
diff --git a/lisp/ob-exp.el b/lisp/ob-exp.el
index 80eaeeb27..a61b26ed5 100644
--- a/lisp/ob-exp.el
+++ b/lisp/ob-exp.el
@@ -419,7 +419,7 @@ replaced with its value."
   (setf (nth 1 info)
 	(if (string= "strip-export" (cdr (assq :noweb (nth 2 info
 	(replace-regexp-in-string
-	 (org-babel-noweb-wrap) "" (nth 1 info))
+	 (org-babel-noweb-make-regexp) "" (nth 1 info))
 	  (if (org-babel-noweb-p (nth 2 info) :export)
 	  (org-babel-expand-noweb-references
 	   info org-babel-exp-reference-buffer)
diff --git a/lisp/ob-tangle.el b/lisp/ob-tangle.el
index 79fe6448b..4427250ae 100644
--- a/lisp/ob-tangle.el
+++ b/lisp/ob-tangle.el
@@ -412,7 +412,7 @@ references."
   (interactive)
   (goto-char (point-min))
   (while (or (re-search-forward "\\[\\[file:.*\\]\\[.*\\]\\]" nil t)
- (re-search-forward (org-babel-noweb-wrap) nil t))
+ (re-search-forward (org-babel-noweb-make-regexp) nil t))
 (delete-region (save-excursion (forward-line) (point))
(save-excursion (end-of-line 1) (forward-char 1) (point)
 
@@ -580,7 +580,8 @@ non-nil, return the full association list to be used by
 	  ;; Run the tangle-body-hook.
   (let ((body (if (org-babel-noweb-p params :tangle)
   (if (string= 

Re: [FR] :noweb-wrap header arg

[Ihor Radchenko]

Amy Grinn  writes:

>>> +(while (< (point) (point-max))
>>> +  (unless (looking-at " *\"\\([^\"]+\\)\" *")
>>> +(looking-at " *\\([^ ]+\\)"))
>>
>> May you please explain the rationale behind this regexp? AFAIU, it
>> implies that you want to allow whitespace characters inside :noweb-wrap
>> boundaries. But I do not think that we need to complicate things so much.
>
> That is exactly what I was going for.  I thought about the ways this
> could be used and the most general-purpose, non-syntax-breaking,
> easily-recognizable way I could think of was to use the language's
> line-comment construct followed by the standard << >> characters.
>
> # <>
> ;; <>
> // <>
>
> I can see how it might be harder to maintain to allow whitespace in the
> noweb-wrap header arg.  I could create a separate org-parse-arg-list
> function to ease that burden somewhat.  My opinion is that having the
> option to use the examples above is preferable to using non-standard
> wrappers, from a third-person point-of-view.

Makes sense. Also, see a similar idea being discussed in
https://list.orgmode.org/orgmode/87o7jlzxgn.fsf@localhost/

I recommend the following:

If the value starts from ", use Elisp's `read':
   |"# <<" ">>"
   (read (current-buffer)) ; => "# <<"
   otherwise, consider read until the first whitespace.
   |#<<; >>;
   (re-search-forward (rx (1+ (not whitespace
   #<<;|
   
However, there may be edge cases like

"<< >>"
"<< >>
<< << >>
<< "asd" >>

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 

Re: [FR] :noweb-wrap header arg

[Amy Grinn]

Ihor Radchenko  writes:

> Amy Grinn  writes:
>
>> +(insert raw)
>> +(goto-char (point-min))
>> +(while (< (point) (point-max))
>> +  (unless (looking-at " *\"\\([^\"]+\\)\" *")
>> +(looking-at " *\\([^ ]+\\)"))
>
> May you please explain the rationale behind this regexp? AFAIU, it
> implies that you want to allow whitespace characters inside :noweb-wrap
> boundaries. But I do not think that we need to complicate things so much.

That is exactly what I was going for.  I thought about the ways this
could be used and the most general-purpose, non-syntax-breaking,
easily-recognizable way I could think of was to use the language's
line-comment construct followed by the standard << >> characters.

# <>
;; <>
// <>

I can see how it might be harder to maintain to allow whitespace in the
noweb-wrap header arg.  I could create a separate org-parse-arg-list
function to ease that burden somewhat.  My opinion is that having the
option to use the examples above is preferable to using non-standard
wrappers, from a third-person point-of-view.

Re: [FR] :noweb-wrap header arg

[Ihor Radchenko]

Amy Grinn  writes:

> First of all, I would like to change (defalias) the function name
> org-babel-noweb-wrap to org-babel-noweb-make-regexp. I think this in
> more in line with other functions which create regular expressions.

+1
You may even use obsolete alias (add it to lisp/org-compat.el)

> Second, the command org-babel-tangle-clean is not able to determine
> which noweb syntax is being used in any tangled source file because the
> header arguments are not tangled along with the source code.
>
> My proposal is to add an additional warning to this command, stating:
>
> """
> Warning, this command removes any lines containing constructs which
> resemble Org file links or noweb references.  It also cannot determine
> which noweb syntax is being used for any given source file, if
> :noweb-wrap was specified in the original Org file.
> """

Makes sense.
Ideally, this function should try to follow the link and lookup code
block headers, but it is already short of doing this. Improving
`org-babel-tangle-clean' is out of scope of your patch.

> +(defun org-babel-get-noweb-wrap ( info)
> +  "Retrieve a description the :noweb-wrap header arg from INFO.
> +
> +The description will be in the form of a list of two of strings
> +for the start and end of a reference.  INFO can be the result of
> +`org-babel-get-src-block-info' otherwise this function will parse
> +info at point."
> +  (unless info
> +(setq info (org-babel-get-src-block-info 'no-eval)))
> +  (when-let ((raw (cdr (assq :noweb-wrap (nth 2 info)
> +(let (result)
> +  (with-temp-buffer

Please, avoid creating throwaway buffers and work with strings instead.
Creating buffers (even temporary buffers) may be costly for some users
who heavily customized buffer hooks.

> +(insert raw)
> +(goto-char (point-min))
> +(while (< (point) (point-max))
> +  (unless (looking-at " *\"\\([^\"]+\\)\" *")
> +(looking-at " *\\([^ ]+\\)"))

May you please explain the rationale behind this regexp? AFAIU, it
implies that you want to allow whitespace characters inside :noweb-wrap
boundaries. But I do not think that we need to complicate things so much.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 

[FR] :noweb-wrap header arg

[Amy Grinn]

Hi!

I'm working on the :noweb-wrap header argument which controls the syntax
of noweb references in a babel src block. For example:

#+name: foo
#+begin_src elisp
  :foo
#+end_src

#+begin_src elisp :noweb yes :noweb-wrap <<< >>>
  <<>>
#+end_src

And I would like some feedback...

First of all, I would like to change (defalias) the function name
org-babel-noweb-wrap to org-babel-noweb-make-regexp. I think this in
more in line with other functions which create regular expressions.

The new way to retrieve the regular expression matching a noweb
reference in the source block at point would be:

(org-babel-noweb-make-regexp nil (org-babel-get-noweb-wrap info))

Where info can be nil or the result of calling
(org-babel-get-src-block-info).

Second, the command org-babel-tangle-clean is not able to determine
which noweb syntax is being used in any tangled source file because the
header arguments are not tangled along with the source code.

My proposal is to add an additional warning to this command, stating:

"""
Warning, this command removes any lines containing constructs which
resemble Org file links or noweb references.  It also cannot determine
which noweb syntax is being used for any given source file, if
:noweb-wrap was specified in the original Org file.
"""

Best,

Amy

>From 1dc8aebcc45447d3b5b38ea3c7700ae2b2686c9d Mon Sep 17 00:00:00 2001
From: Amy Grinn 
Date: Mon, 8 Apr 2024 09:05:02 -0400
Subject: [PATCH] (WIP) lisp/ob-core.el: New :noweb-wrap header arg

* lisp/ob-core: (org-babel-noweb-wrap): Add optional third parameter
'wrap'.
* lisp/ob-core: (org-babel-get-noweb-wrap): New function for parsing
:noweb-wrap header arg.
* etc/ORG-NEWS (New =:noweb-wrap= babel header argument): Describe new
argument.
* others...
---
 etc/ORG-NEWS| 14 ++
 lisp/ob-core.el | 51 --
 lisp/ob-exp.el  |  3 +-
 lisp/ob-tangle.el   |  6 +++-
 testing/examples/babel.org  | 17 
 testing/lisp/test-ob-exp.el | 55 +
 testing/lisp/test-ob.el | 15 ++
 7 files changed, 150 insertions(+), 11 deletions(-)

diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index aeb7ffd4b..162e7f035 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -621,6 +621,20 @@ link when storing any type of external link type in an Org file, not
 just =id:= links.
 
 ** New and changed options
+*** New =:noweb-wrap= babel header argument
+
+This argument changes the default noweb reference syntax by masking
+the options ~org-babel-noweb-wrap-start~ and
+~org-babel-noweb-wrap-end~.
+
+=:noweb-wrap= takes two parameters, start and end, corresponding to
+each option.
+
+For example:
+: #+begin_src sh :noweb-wrap <<< >>>
+:   echo <<>>
+: #+end_src
+
 *** =.avif= images are now recognized in ~org-html-inline-image-rules~
 
 In =ox-html=, =.avif= image links are now inlined by default.
diff --git a/lisp/ob-core.el b/lisp/ob-core.el
index 8dfc07a4e..843794322 100644
--- a/lisp/ob-core.el
+++ b/lisp/ob-core.el
@@ -194,15 +194,21 @@ This string must include a \"%s\" which will be replaced by the results."
   :package-version '(Org . "9.1")
   :safe #'booleanp)
 
-(defun org-babel-noweb-wrap ( regexp)
+(defun org-babel-noweb-wrap ( regexp wrap)
   "Return regexp matching a Noweb reference.
 
 Match any reference, or only those matching REGEXP, if non-nil.
 
+If WRAP is provided, it should be a list of 2 strings describing
+the start and end of a noweb reference, such as that returned by
+`org-babel-get-noweb-wrap'.  Otherwise
+`org-babel-noweb-wrap-start' and `org-babel-noweb-wrap-end' will
+be used.
+
 When matching, reference is stored in match group 1."
-  (concat (regexp-quote org-babel-noweb-wrap-start)
-	  (or regexp "\\([^ \t\n]\\(?:.*?[^ \t\n]\\)?\\)")
-	  (regexp-quote org-babel-noweb-wrap-end)))
+  (concat (regexp-quote (or (car wrap) org-babel-noweb-wrap-start))
+	(or regexp "\\([^ \t\n]\\(?:.*?[^ \t\n]\\)?\\)")
+	(regexp-quote (or (cadr wrap) org-babel-noweb-wrap-end
 
 (defvar org-babel-src-name-regexp
   "^[ \t]*#\\+name:[ \t]*"
@@ -1963,6 +1969,27 @@ src block, then return nil."
   (let ((head (org-babel-where-is-src-block-head)))
 (if head (goto-char head) (error "Not currently in a code block"
 
+(defun org-babel-get-noweb-wrap ( info)
+  "Retrieve a description the :noweb-wrap header arg from INFO.
+
+The description will be in the form of a list of two of strings
+for the start and end of a reference.  INFO can be the result of
+`org-babel-get-src-block-info' otherwise this function will parse
+info at point."
+  (unless info
+(setq info (org-babel-get-src-block-info 'no-eval)))
+  (when-let ((raw (cdr (assq :noweb-wrap (nth 2 info)
+(let (result)
+  (with-temp-buffer
+(insert raw)
+(goto-char (point-min))
+(while (< (point) (point-max))
+  (unless (looking-at " *\"\\([^\"]+\\)\" *")
+(looking-at " *\\([^ ]+\\)"))
+