emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* [PATCH] Add light argument to org-babel-lob-get-info
@ 2022-10-16 12:17 Ferdinand Pieper
  2022-10-17  9:10 ` Ihor Radchenko
  0 siblings, 1 reply; 10+ messages in thread
From: Ferdinand Pieper @ 2022-10-16 12:17 UTC (permalink / raw)
  To: emacs-orgmode

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

Similar to ~org-babel-get-src-block-info~ it is sometimes useful to disable evaluation of lisp parameters when getting the info of a lob call. This patch adds an argument for that.

Better name for the argument could be ~no-eval~, but I decided to stick with the naming in ~org-babel-get-src-block-info~. To be completely consistent with ~org-babel-get-src-block-info~ the argument order could be swapped, but this would break existing function calls. 

What do you think?

Best,
Ferdinand


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org-babel-lob-get-info-Add-light-argument.patch --]
[-- Type: text/x-diff, Size: 3134 bytes --]

From ba8069a3b83489ee1de8c4eeba059883809d0ea7 Mon Sep 17 00:00:00 2001
From: fpi <git@pie.tf>
Date: Sun, 16 Oct 2022 13:17:40 +0200
Subject: [PATCH] org-babel-lob-get-info: Add light argument

* lisp/ob-lob.el (org-babel-lob-get-info): Add light argument to
prevent recursive evaluation of lisp values in parameters.
---
 lisp/ob-exp.el |  2 +-
 lisp/ob-lob.el | 13 +++++++++----
 lisp/ob-ref.el |  2 +-
 3 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/lisp/ob-exp.el b/lisp/ob-exp.el
index e9b304b86..83dd5fc74 100644
--- a/lisp/ob-exp.el
+++ b/lisp/ob-exp.el
@@ -25,7 +25,7 @@
 ;;; Code:
 (require 'ob-core)
 
-(declare-function org-babel-lob-get-info "ob-lob" (&optional datum))
+(declare-function org-babel-lob-get-info "ob-lob" (&optional datum light))
 (declare-function org-element-at-point "org-element" ())
 (declare-function org-element-context "org-element" (&optional element))
 (declare-function org-element-property "org-element" (property element))
diff --git a/lisp/ob-lob.el b/lisp/ob-lob.el
index 903dabfbd..3043ff647 100644
--- a/lisp/ob-lob.el
+++ b/lisp/ob-lob.el
@@ -114,11 +114,16 @@ after REF in the Library of Babel."
 	    (cdr (assoc-string ref org-babel-library-of-babel))))))))
 
 ;;;###autoload
-(defun org-babel-lob-get-info (&optional datum)
+(defun org-babel-lob-get-info (&optional datum light)
   "Return internal representation for Library of Babel function call.
 
 Consider DATUM, when provided, or element at point otherwise.
 
+When optional argument LIGHT is non-nil, Babel does not resolve
+remote variable references; a process which could likely result
+in the execution of other code blocks, and do not evaluate Lisp
+values in parameters.
+
 Return nil when not on an appropriate location.  Otherwise return
 a list compatible with `org-babel-get-src-block-info', which
 see."
@@ -139,16 +144,16 @@ see."
 			org-babel-default-lob-header-args
 			(append
 			 (org-with-point-at begin
-			   (org-babel-params-from-properties language))
+			   (org-babel-params-from-properties language light))
 			 (list
 			  (org-babel-parse-header-arguments
-			   (org-element-property :inside-header context))
+			   (org-element-property :inside-header context) light)
 			  (let ((args (org-element-property :arguments context)))
 			    (and args
 				 (mapcar (lambda (ref) (cons :var ref))
 					 (org-babel-ref-split-args args))))
 			  (org-babel-parse-header-arguments
-			   (org-element-property :end-header context)))))
+			   (org-element-property :end-header context) light))))
 		 nil
 		 (org-element-property :name context)
 		 begin
diff --git a/lisp/ob-ref.el b/lisp/ob-ref.el
index a7ab299b2..a7cdb22e1 100644
--- a/lisp/ob-ref.el
+++ b/lisp/ob-ref.el
@@ -53,7 +53,7 @@
 (require 'org-macs)
 (require 'cl-lib)
 
-(declare-function org-babel-lob-get-info "ob-lob" (&optional datum))
+(declare-function org-babel-lob-get-info "ob-lob" (&optional datum light))
 (declare-function org-element-at-point "org-element" ())
 (declare-function org-element-property "org-element" (property element))
 (declare-function org-element-type "org-element" (element))
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] Add light argument to org-babel-lob-get-info
  2022-10-16 12:17 [PATCH] Add light argument to org-babel-lob-get-info Ferdinand Pieper
@ 2022-10-17  9:10 ` Ihor Radchenko
  2022-10-18 16:15   ` Ferdinand Pieper
  0 siblings, 1 reply; 10+ messages in thread
From: Ihor Radchenko @ 2022-10-17  9:10 UTC (permalink / raw)
  To: Ferdinand Pieper; +Cc: emacs-orgmode

Ferdinand Pieper <fer@pie.tf> writes:

> Similar to ~org-babel-get-src-block-info~ it is sometimes useful to disable evaluation of lisp parameters when getting the info of a lob call. This patch adds an argument for that.
>
> Better name for the argument could be ~no-eval~, but I decided to stick with the naming in ~org-babel-get-src-block-info~. To be completely consistent with ~org-babel-get-src-block-info~ the argument order could be swapped, but this would break existing function calls. 
>
> What do you think?

I see no problem with this addition.

I'd prefer to change LIGHT to NO-EVAL, including in
org-babel-get-src-block-info. Changing argument name in function does
not affect its caller in any way. Just need to update the function body
and docstring carefully.

NO-EVAL is already used by org-babel-parse-header-arguments and
org-babel-params-from-properties.

May I know if you are proposing this for a specific purpose?

-- 
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>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Add light argument to org-babel-lob-get-info
  2022-10-17  9:10 ` Ihor Radchenko
@ 2022-10-18 16:15   ` Ferdinand Pieper
  2022-10-19  7:01     ` Ihor Radchenko
  0 siblings, 1 reply; 10+ messages in thread
From: Ferdinand Pieper @ 2022-10-18 16:15 UTC (permalink / raw)
  To: Ihor Radchenko; +Cc: emacs-orgmode

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

Ihor Radchenko <yantar92@posteo.net> writes:

> I'd prefer to change LIGHT to NO-EVAL, including in
> org-babel-get-src-block-info. Changing argument name in function does
> not affect its caller in any way. Just need to update the function body
> and docstring carefully.
>
> NO-EVAL is already used by org-babel-parse-header-arguments and
> org-babel-params-from-properties.

I also prefer the NO-EVAL naming. I updated the patch to use no-eval and also added a second patch to rename the light argument of org-babel-get-src-block-info and all its occurrences (I changed the 'light to 'no-eval in all its calls only for consistency and future proofing reasons, altough it is unnecessary right now). Feel free to squash them into one commit. I was not sure wether one or two commits would be preferred.

> May I know if you are proposing this for a specific purpose?

Thanks for asking. I use it in a function that creates quasi unique names similar to org-babel-temp-file. But instead of random names it uses the source block content and header info to create a unique hash. This makes it very easy to create unique but from call to call consistent filenames in header arguments, when you don't want to worry about the filename, but also don't want to clutter your result directory (especially if its not /tmp) or care about constant filenames, e.g. for linking. Without the no-eval/light argument the hash generation gets stuck in a recursive loop. I attached the functions for reference.
If there is wider interest in this, these could be added to org either as a variant of org-babel-temp-file or as an extension.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org-babel-lob-get-info-Add-no-eval-argument.patch --]
[-- Type: text/x-diff, Size: 3191 bytes --]

From 22edd4f3a9382eb3dca5558b13053cba6fedf7ce Mon Sep 17 00:00:00 2001
From: fpi <git@pie.tf>
Date: Tue, 18 Oct 2022 17:27:54 +0200
Subject: [PATCH 1/2] org-babel-lob-get-info: Add no-eval argument

* lisp/ob-lob.el (org-babel-lob-get-info): Add no-eval argument to
prevent recursive evaluation of lisp values in parameters.
---
 lisp/ob-exp.el |  2 +-
 lisp/ob-lob.el | 13 +++++++++----
 lisp/ob-ref.el |  2 +-
 3 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/lisp/ob-exp.el b/lisp/ob-exp.el
index de7e1e3a3..67ed22f84 100644
--- a/lisp/ob-exp.el
+++ b/lisp/ob-exp.el
@@ -29,7 +29,7 @@
 
 (require 'ob-core)
 
-(declare-function org-babel-lob-get-info "ob-lob" (&optional datum))
+(declare-function org-babel-lob-get-info "ob-lob" (&optional datum no-eval))
 (declare-function org-element-at-point "org-element" (&optional pom cached-only))
 (declare-function org-element-context "org-element" (&optional element))
 (declare-function org-element-property "org-element" (property element))
diff --git a/lisp/ob-lob.el b/lisp/ob-lob.el
index c6be8be80..10822accb 100644
--- a/lisp/ob-lob.el
+++ b/lisp/ob-lob.el
@@ -119,10 +119,15 @@ after REF in the Library of Babel."
 	    (cdr (assoc-string ref org-babel-library-of-babel))))))))
 
 ;;;###autoload
-(defun org-babel-lob-get-info (&optional datum)
+(defun org-babel-lob-get-info (&optional datum no-eval)
   "Return internal representation for Library of Babel function call.
 
 Consider DATUM, when provided, or element at point otherwise.
+ 
+When optional argument NO-EVAL is non-nil, Babel does not resolve
+remote variable references; a process which could likely result
+in the execution of other code blocks, and do not evaluate Lisp
+values in parameters.
 
 Return nil when not on an appropriate location.  Otherwise return
 a list compatible with `org-babel-get-src-block-info', which
@@ -144,16 +149,16 @@ see."
 			org-babel-default-lob-header-args
 			(append
 			 (org-with-point-at begin
-			   (org-babel-params-from-properties language))
+			   (org-babel-params-from-properties language no-eval))
 			 (list
 			  (org-babel-parse-header-arguments
-			   (org-element-property :inside-header context))
+			   (org-element-property :inside-header context) no-eval)
 			  (let ((args (org-element-property :arguments context)))
 			    (and args
 				 (mapcar (lambda (ref) (cons :var ref))
 					 (org-babel-ref-split-args args))))
 			  (org-babel-parse-header-arguments
-			   (org-element-property :end-header context)))))
+			   (org-element-property :end-header context) no-eval))))
 		 nil
 		 (org-element-property :name context)
 		 begin
diff --git a/lisp/ob-ref.el b/lisp/ob-ref.el
index 2b4a16aea..2bba2071e 100644
--- a/lisp/ob-ref.el
+++ b/lisp/ob-ref.el
@@ -57,7 +57,7 @@
 (require 'org-macs)
 (require 'cl-lib)
 
-(declare-function org-babel-lob-get-info "ob-lob" (&optional datum))
+(declare-function org-babel-lob-get-info "ob-lob" (&optional datum no-eval))
 (declare-function org-element-at-point "org-element" (&optional pom cached-only))
 (declare-function org-element-property "org-element" (property element))
 (declare-function org-element-type "org-element" (element))
-- 
2.20.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-org-babel-get-src-block-info-Rename-light-argument.patch --]
[-- Type: text/x-diff, Size: 8607 bytes --]

From 015ce9ef56c997331ea34957d47fa98efffb54a3 Mon Sep 17 00:00:00 2001
From: fpi <git@pie.tf>
Date: Tue, 18 Oct 2022 17:34:18 +0200
Subject: [PATCH 2/2] org-babel-get-src-block-info: Rename light argument

* lisp/ob-core.el (org-babel-get-src-block-info): Rename argument
light to no-eval.
---
 lisp/ob-core.el       | 20 ++++++++++----------
 lisp/ob-lob.el        |  2 +-
 lisp/ob-tangle.el     |  6 +++---
 lisp/org-pcomplete.el |  4 ++--
 lisp/org-src.el       |  4 ++--
 5 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/lisp/ob-core.el b/lisp/ob-core.el
index f273fa92e..c725a5508 100644
--- a/lisp/ob-core.el
+++ b/lisp/ob-core.el
@@ -339,7 +339,7 @@ then run `org-babel-execute-src-block'."
 This includes header arguments, language and name, and is largely
 a window into the `org-babel-get-src-block-info' function."
   (interactive)
-  (let ((info (org-babel-get-src-block-info 'light))
+  (let ((info (org-babel-get-src-block-info 'no-eval))
 	(full (lambda (it) (> (length it) 0)))
 	(printf (lambda (fmt &rest args) (princ (apply #'format fmt args)))))
     (when info
@@ -640,10 +640,10 @@ the list of header arguments."
         (push elem lst)))
     (reverse lst)))
 
-(defun org-babel-get-src-block-info (&optional light datum)
+(defun org-babel-get-src-block-info (&optional no-eval datum)
   "Extract information from a source block or inline source block.
 
-When optional argument LIGHT is non-nil, Babel does not resolve
+When optional argument NO-EVAL is non-nil, Babel does not resolve
 remote variable references; a process which could likely result
 in the execution of other code blocks, and do not evaluate Lisp
 values in parameters.
@@ -677,9 +677,9 @@ a list with the following pattern:
 		       ;; properties applicable to its location within
 		       ;; the document.
 		       (org-with-point-at (org-element-property :begin datum)
-			 (org-babel-params-from-properties lang light))
+			 (org-babel-params-from-properties lang no-eval))
 		       (mapcar (lambda (h)
-				 (org-babel-parse-header-arguments h light))
+				 (org-babel-parse-header-arguments h no-eval))
 			       (cons (org-element-property :parameters datum)
 				     (org-element-property :header datum)))))
 	       (or (org-element-property :switches datum) "")
@@ -687,7 +687,7 @@ a list with the following pattern:
 	       (org-element-property (if inline :begin :post-affiliated)
 				     datum)
 	       (and (not inline) (org-src-coderef-format datum)))))
-	(unless light
+	(unless no-eval
 	  (setf (nth 2 info) (org-babel-process-params (nth 2 info))))
 	(setf (nth 2 info) (org-babel-generate-file-param name (nth 2 info)))
 	info))))
@@ -933,7 +933,7 @@ arguments and pop open the results in a preview buffer."
 (defun org-babel-insert-header-arg (&optional header-arg value)
   "Insert a header argument selecting from lists of common args and values."
   (interactive)
-  (let* ((info (org-babel-get-src-block-info 'light))
+  (let* ((info (org-babel-get-src-block-info 'no-eval))
 	 (lang (car info))
 	 (begin (nth 5 info))
 	 (lang-headers (intern (concat "org-babel-header-args:" lang)))
@@ -1130,7 +1130,7 @@ code block, otherwise return nil.  With optional prefix argument
 RE-RUN the source-code block is evaluated even if results already
 exist."
   (interactive "P")
-  (pcase (org-babel-get-src-block-info 'light)
+  (pcase (org-babel-get-src-block-info 'no-eval)
     (`(,_ ,_ ,arguments ,_ ,_ ,start ,_)
      (save-excursion
        ;; Go to the results, if there aren't any then run the block.
@@ -1967,7 +1967,7 @@ split.  When called from outside of a code block a new code block
 is created.  In both cases if the region is demarcated and if the
 region is not active then the point is demarcated."
   (interactive "P")
-  (let* ((info (org-babel-get-src-block-info 'light))
+  (let* ((info (org-babel-get-src-block-info 'no-eval))
 	 (start (org-babel-where-is-src-block-head))
 	 (block (and start (match-string 0)))
 	 (headers (and start (match-string 4)))
@@ -2916,7 +2916,7 @@ would set the value of argument \"a\" equal to \"9\".  Note that
 these arguments are not evaluated in the current source-code
 block but are passed literally to the \"example-block\"."
   (let* ((parent-buffer (or parent-buffer (current-buffer)))
-	 (info (or info (org-babel-get-src-block-info 'light)))
+	 (info (or info (org-babel-get-src-block-info 'no-eval)))
          (lang (nth 0 info))
          (body (nth 1 info))
 	 (comment (string= "noweb" (cdr (assq :comments (nth 2 info)))))
diff --git a/lisp/ob-lob.el b/lisp/ob-lob.el
index 10822accb..e4766a7d1 100644
--- a/lisp/ob-lob.el
+++ b/lisp/ob-lob.el
@@ -54,7 +54,7 @@ should not be inherited from a source block.")
   (interactive "fFile: ")
   (let ((lob-ingest-count 0))
     (org-babel-map-src-blocks file
-      (let* ((info (org-babel-get-src-block-info 'light))
+      (let* ((info (org-babel-get-src-block-info 'no-eval))
 	     (source-name (nth 4 info)))
 	(when source-name
 	  (setf (nth 1 info)
diff --git a/lisp/ob-tangle.el b/lisp/ob-tangle.el
index 3240b994e..2da92efaf 100644
--- a/lisp/ob-tangle.el
+++ b/lisp/ob-tangle.el
@@ -246,7 +246,7 @@ matching a regular expression."
 	       org-babel-default-header-args))
 	    (tangle-file
 	     (when (equal arg '(16))
-	       (or (cdr (assq :tangle (nth 2 (org-babel-get-src-block-info 'light))))
+	       (or (cdr (assq :tangle (nth 2 (org-babel-get-src-block-info 'no-eval))))
 		   (user-error "Point is not in a source code block"))))
 	    path-collector)
 	(mapc ;; map over file-names
@@ -461,7 +461,7 @@ code blocks by target file."
 	  (setq last-heading-pos current-heading-pos)))
       (unless (or (org-in-commented-heading-p)
 		  (org-in-archived-heading-p))
-	(let* ((info (org-babel-get-src-block-info 'light))
+	(let* ((info (org-babel-get-src-block-info 'no-eval))
 	       (src-lang (nth 0 info))
 	       (src-tfile (cdr (assq :tangle (nth 2 info)))))
 	  (unless (or (string= src-tfile "no")
@@ -594,7 +594,7 @@ non-nil, return the full association list to be used by
   "Return a list of begin and end link comments for the code block at point.
 INFO, when non nil, is the source block information, as returned
 by `org-babel-get-src-block-info'."
-  (let ((link-data (pcase (or info (org-babel-get-src-block-info 'light))
+  (let ((link-data (pcase (or info (org-babel-get-src-block-info 'no-eval))
 		     (`(,_ ,_ ,params ,_ ,name ,start ,_)
 		      `(("start-line" . ,(org-with-point-at start
 					   (number-to-string
diff --git a/lisp/org-pcomplete.el b/lisp/org-pcomplete.el
index 225cdc093..14bdc55e9 100644
--- a/lisp/org-pcomplete.el
+++ b/lisp/org-pcomplete.el
@@ -35,7 +35,7 @@
 
 (declare-function org-at-heading-p "org" (&optional ignored))
 (declare-function org-babel-combine-header-arg-lists "ob-core" (original &rest others))
-(declare-function org-babel-get-src-block-info "ob-core" (&optional light datum))
+(declare-function org-babel-get-src-block-info "ob-core" (&optional no-eval datum))
 (declare-function org-before-first-heading-p "org" ())
 (declare-function org-buffer-property-keys "org" (&optional specials defaults columns))
 (declare-function org-element-at-point "org-element" (&optional pom cached-only))
@@ -428,7 +428,7 @@ switches."
 				    (symbol-plist
 				     'org-babel-load-languages)
 				    'custom-type)))))))
-  (let* ((info (org-babel-get-src-block-info 'light))
+  (let* ((info (org-babel-get-src-block-info 'no-eval))
 	 (lang (car info))
 	 (lang-headers (intern (concat "org-babel-header-args:" lang)))
 	 (headers (org-babel-combine-header-arg-lists
diff --git a/lisp/org-src.el b/lisp/org-src.el
index 2c1dd98ea..8744e98bf 100644
--- a/lisp/org-src.el
+++ b/lisp/org-src.el
@@ -1250,7 +1250,7 @@ name of the sub-editing buffer."
 	      "example"))
 	   (lang-f (and (eq type 'src-block) (org-src-get-lang-mode lang)))
 	   (babel-info (and (eq type 'src-block)
-			    (org-babel-get-src-block-info 'light)))
+			    (org-babel-get-src-block-info 'no-eval)))
 	   deactivate-mark)
       (when (and (eq type 'src-block) (not (functionp lang-f)))
 	(error "No such language mode: %s" lang-f))
@@ -1282,7 +1282,7 @@ name of the sub-editing buffer."
       (user-error "Not on inline source code"))
     (let* ((lang (org-element-property :language context))
 	   (lang-f (org-src-get-lang-mode lang))
-	   (babel-info (org-babel-get-src-block-info 'light))
+	   (babel-info (org-babel-get-src-block-info 'no-eval))
 	   deactivate-mark)
       (unless (functionp lang-f) (error "No such language mode: %s" lang-f))
       (org-src--edit-element
-- 
2.20.1


[-- Attachment #4: org-babel-src-block-temp-file.el --]
[-- Type: application/emacs-lisp, Size: 2626 bytes --]

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] Add light argument to org-babel-lob-get-info
  2022-10-18 16:15   ` Ferdinand Pieper
@ 2022-10-19  7:01     ` Ihor Radchenko
  2022-10-22 15:28       ` Possible bugs in org-babel-temp-stable-file (was: [PATCH] Add light argument to org-babel-lob-get-info) Ferdinand Pieper
  0 siblings, 1 reply; 10+ messages in thread
From: Ihor Radchenko @ 2022-10-19  7:01 UTC (permalink / raw)
  To: Ferdinand Pieper; +Cc: emacs-orgmode

Ferdinand Pieper <fer@pie.tf> writes:

> Ihor Radchenko <yantar92@posteo.net> writes:
>
>> I'd prefer to change LIGHT to NO-EVAL, including in
>> org-babel-get-src-block-info. Changing argument name in function does
>> not affect its caller in any way. Just need to update the function body
>> and docstring carefully.
>>
>> NO-EVAL is already used by org-babel-parse-header-arguments and
>> org-babel-params-from-properties.
>
> I also prefer the NO-EVAL naming. I updated the patch to use no-eval and also added a second patch to rename the light argument of org-babel-get-src-block-info and all its occurrences (I changed the 'light to 'no-eval in all its calls only for consistency and future proofing reasons, altough it is unnecessary right now). Feel free to squash them into one commit. I was not sure wether one or two commits would be preferred.

Thanks!
Applied onto main.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=bbec9aafee8f0cd022a8e2b782ac1f3f920fdb8f
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=e58bd039e3c885cc3a3e6fd422b5a06a9ad1eea4

>> May I know if you are proposing this for a specific purpose?
>
> Thanks for asking. I use it in a function that creates quasi unique names similar to org-babel-temp-file. But instead of random names it uses the source block content and header info to create a unique hash. This makes it very easy to create unique but from call to call consistent filenames in header arguments, when you don't want to worry about the filename, but also don't want to clutter your result directory (especially if its not /tmp) or care about constant filenames, e.g. for linking. Without the no-eval/light argument the hash generation gets stuck in a recursive loop. I attached the functions for reference.
> If there is wider interest in this, these could be added to org either as a variant of org-babel-temp-file or as an extension.

Have you seen org-babel-temp-stable-file?

-- 
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>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Possible bugs in org-babel-temp-stable-file (was: [PATCH] Add light argument to org-babel-lob-get-info)
  2022-10-19  7:01     ` Ihor Radchenko
@ 2022-10-22 15:28       ` Ferdinand Pieper
  2022-10-23  4:01         ` Ihor Radchenko
  0 siblings, 1 reply; 10+ messages in thread
From: Ferdinand Pieper @ 2022-10-22 15:28 UTC (permalink / raw)
  To: Ihor Radchenko; +Cc: emacs-orgmode

Ihor Radchenko <yantar92@posteo.net> writes:

> Applied onto main.

Thanks

> Have you seen org-babel-temp-stable-file?

I have not. Seems really useful, thanks for mentioning. I wrote my functions before org-babel-temp-stable-file was added, but seems I can simplify them now quite a bit.

I noticed two possible bugs in org-babel-temp-stable-file:

1. Prefix could be an empty string and that would break the filename extension, because (expand-file-name "" org-babel-temporary-stable-directory) would not add a slash in the path.

2. The org-babel-temporary-stable-directory is only created upon initial loading. But the intended behavior for remote paths seems to be to create the temp files on the remote. Which will fail, because org-babel-temporary-stable-directory does not exist on the remote. However creating the directories on remotes of course makes cleanup harder to impossible, if the remotes are not accessible any more.

I can try to write a patch for both, but I do not fully understand the org-babel-temp-stable-file code. It seems with the use of with-temp-file there is no need to change temporary-file-directory anymore?

It could also be nice to add an extra argument to force the use of the local org-babel-temporary-stable-directory for remote files.


Best,


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Possible bugs in org-babel-temp-stable-file (was: [PATCH] Add light argument to org-babel-lob-get-info)
  2022-10-22 15:28       ` Possible bugs in org-babel-temp-stable-file (was: [PATCH] Add light argument to org-babel-lob-get-info) Ferdinand Pieper
@ 2022-10-23  4:01         ` Ihor Radchenko
  2022-10-23 11:27           ` Possible bugs in org-babel-temp-stable-file Ferdinand Pieper
  0 siblings, 1 reply; 10+ messages in thread
From: Ihor Radchenko @ 2022-10-23  4:01 UTC (permalink / raw)
  To: Ferdinand Pieper; +Cc: emacs-orgmode

Ferdinand Pieper <fer@pie.tf> writes:

>> Have you seen org-babel-temp-stable-file?
>
> I have not. Seems really useful, thanks for mentioning. I wrote my functions before org-babel-temp-stable-file was added, but seems I can simplify them now quite a bit.
>
> I noticed two possible bugs in org-babel-temp-stable-file:
>
> 1. Prefix could be an empty string and that would break the filename extension, because (expand-file-name "" org-babel-temporary-stable-directory) would not add a slash in the path.

Fixed now.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=69e3a4db3d0c54b4165761f56523da4962eff74c

> 2. The org-babel-temporary-stable-directory is only created upon initial loading. But the intended behavior for remote paths seems to be to create the temp files on the remote. Which will fail, because org-babel-temporary-stable-directory does not exist on the remote. However creating the directories on remotes of course makes cleanup harder to impossible, if the remotes are not accessible any more.

For remotes, we fall back to org-babel-remote-temporary-directory.
Apparently in all but one place in code. Now fixed.

> It could also be nice to add an extra argument to force the use of the local org-babel-temporary-stable-directory for remote files.

I am not sure if it is a good idea.
On remote files, default-directory often points to the remote making
`shell-command' and similar run code on remote machine. It may not work
well with local temporary directory.

-- 
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>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Possible bugs in org-babel-temp-stable-file
  2022-10-23  4:01         ` Ihor Radchenko
@ 2022-10-23 11:27           ` Ferdinand Pieper
  2022-10-24  4:12             ` Ihor Radchenko
  0 siblings, 1 reply; 10+ messages in thread
From: Ferdinand Pieper @ 2022-10-23 11:27 UTC (permalink / raw)
  To: Ihor Radchenko; +Cc: emacs-orgmode

Ihor Radchenko <yantar92@posteo.net> writes:

> Fixed now.
> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=69e3a4db3d0c54b4165761f56523da4962eff74c

Seems good, thanks.

>> It could also be nice to add an extra argument to force the use of
>> the local org-babel-temporary-stable-directory for remote files.
>
> I am not sure if it is a good idea.
> On remote files, default-directory often points to the remote making
> `shell-command' and similar run code on remote machine. It may not work
> well with local temporary directory.

I think you are right. Something like this could only be relevant for local files which execute code on remote locations (e.g. babel via the :dir argument) and require remote temp files instead of local ones. But that seems like too much of an edge case to support. If necessary a user could easily solve it with a small wrapper, that sets default-directory accordingly.



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Possible bugs in org-babel-temp-stable-file
  2022-10-23 11:27           ` Possible bugs in org-babel-temp-stable-file Ferdinand Pieper
@ 2022-10-24  4:12             ` Ihor Radchenko
  2022-10-24  7:49               ` Ferdinand Pieper
  0 siblings, 1 reply; 10+ messages in thread
From: Ihor Radchenko @ 2022-10-24  4:12 UTC (permalink / raw)
  To: Ferdinand Pieper; +Cc: emacs-orgmode

Ferdinand Pieper <fer@pie.tf> writes:

>>> It could also be nice to add an extra argument to force the use of
>>> the local org-babel-temporary-stable-directory for remote files.
>>
>> I am not sure if it is a good idea.
>> On remote files, default-directory often points to the remote making
>> `shell-command' and similar run code on remote machine. It may not work
>> well with local temporary directory.
>
> I think you are right. Something like this could only be relevant for local files which execute code on remote locations (e.g. babel via the :dir argument) and require remote temp files instead of local ones. But that seems like too much of an edge case to support. If necessary a user could easily solve it with a small wrapper, that sets default-directory accordingly.

When you have :dir argument in a source block, default-directory is set
to :dir value during execution. This affects the location of temporary
files as well.

-- 
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>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Possible bugs in org-babel-temp-stable-file
  2022-10-24  4:12             ` Ihor Radchenko
@ 2022-10-24  7:49               ` Ferdinand Pieper
  2022-10-24  8:24                 ` Ihor Radchenko
  0 siblings, 1 reply; 10+ messages in thread
From: Ferdinand Pieper @ 2022-10-24  7:49 UTC (permalink / raw)
  To: Ihor Radchenko; +Cc: emacs-orgmode

Ihor Radchenko <yantar92@posteo.net> writes:

> When you have :dir argument in a source block, default-directory is set
> to :dir value during execution. This affects the location of temporary
> files as well.

Yes, but not for header argument evaluation (which I think is reasonable).
An example of what I meant:

--8<---------------cut here---------------start------------->8---
#+begin_src shell :dir /ssh:remote: :var file=(org-babel-temp-stable-file (org-babel-get-src-block-info t) "")
echo hello world > $file
#+end_src
--8<---------------cut here---------------end--------------->8---


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Possible bugs in org-babel-temp-stable-file
  2022-10-24  7:49               ` Ferdinand Pieper
@ 2022-10-24  8:24                 ` Ihor Radchenko
  0 siblings, 0 replies; 10+ messages in thread
From: Ihor Radchenko @ 2022-10-24  8:24 UTC (permalink / raw)
  To: Ferdinand Pieper; +Cc: emacs-orgmode

Ferdinand Pieper <fer@pie.tf> writes:

> Ihor Radchenko <yantar92@posteo.net> writes:
>
>> When you have :dir argument in a source block, default-directory is set
>> to :dir value during execution. This affects the location of temporary
>> files as well.
>
> Yes, but not for header argument evaluation (which I think is reasonable).
> An example of what I meant:
>
> --8<---------------cut here---------------start------------->8---
> #+begin_src shell :dir /ssh:remote: :var file=(org-babel-temp-stable-file (org-babel-get-src-block-info t) "")
> echo hello world > $file
> #+end_src
> --8<---------------cut here---------------end--------------->8---

Sure. I believe that we do not need to change anything wrt this example.
Though maybe explain a bit in documentation? Patches welcome!

-- 
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>


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2022-10-24  9:12 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-16 12:17 [PATCH] Add light argument to org-babel-lob-get-info Ferdinand Pieper
2022-10-17  9:10 ` Ihor Radchenko
2022-10-18 16:15   ` Ferdinand Pieper
2022-10-19  7:01     ` Ihor Radchenko
2022-10-22 15:28       ` Possible bugs in org-babel-temp-stable-file (was: [PATCH] Add light argument to org-babel-lob-get-info) Ferdinand Pieper
2022-10-23  4:01         ` Ihor Radchenko
2022-10-23 11:27           ` Possible bugs in org-babel-temp-stable-file Ferdinand Pieper
2022-10-24  4:12             ` Ihor Radchenko
2022-10-24  7:49               ` Ferdinand Pieper
2022-10-24  8:24                 ` Ihor Radchenko

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).