Hi Ihor and Max,

I've had a bit more spare time to try to make progress on this. I think I've 
addressed all your earlier comments from summer when I last worked on this. I'm 
dropping the feature to inject variables in screen onto remote host 
(integrating Tramp and screen), or adding support for :prologue in ob-screen to 
do Tramp-ish things.

001 fixes org-babel-screen-test so that it passes. 

002 adds support for non-standard screen location, including spaces in paths.

003 adds support for :var header args. Test with sh, bash, and fish shell 
(different syntax). Seems to work.

If you approve, I will push these to main once I have write access to
the repository.

Best,

  Ken Mankoff

>From b78db9c23bd30db9c08c633e7fdce37779290f80 Mon Sep 17 00:00:00 2001
From: Ken Mankoff <[email protected]>
Date: Wed, 23 Jul 2025 08:41:57 -0400
Subject: [PATCH 1/3] lisp/ob-screen.el (org-babel-screen-test): Test now
 passes

* lisp/ob-screen.el (org-babel-screen-test): Test now passes
---
 lisp/ob-screen.el | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/lisp/ob-screen.el b/lisp/ob-screen.el
index 554a01406575..60a473f3978b 100644
--- a/lisp/ob-screen.el
+++ b/lisp/ob-screen.el
@@ -128,16 +128,15 @@ The terminal should shortly flicker."
          (body (concat "echo '" random-string "' > " tmpfile "\nexit\n"))
          tmp-string)
     (org-babel-execute:screen body org-babel-default-header-args:screen)
-    ;; XXX: need to find a better way to do the following
-    (while (not (file-readable-p tmpfile))
-      ;; do something, otherwise this will be optimized away
-      (message "org-babel-screen: File not readable yet."))
+    (while (= (file-attribute-size (file-attributes tmpfile)) 0)
+      (progn (message "org-babel-screen: Still executing...")
+             (sleep-for 0.1)))
     (setq tmp-string (with-temp-buffer
                        (insert-file-contents-literally tmpfile)
                        (buffer-substring (point-min) (point-max))))
     (delete-file tmpfile)
     (message (concat "org-babel-screen: Setup "
-                     (if (string-match random-string tmp-string)
+                     (if (string-match-p random-string tmp-string)
                          "WORKS."
 		       "DOESN'T work.")))))
 
-- 
2.47.3

>From 98d39546e522e4eedbecf14cb3c3e44442e658fa Mon Sep 17 00:00:00 2001
From: Ken Mankoff <[email protected]>
Date: Wed, 23 Jul 2025 08:51:33 -0400
Subject: [PATCH 3/3] lisp/ob-screen.el: Support :var header argument

* lisp/ob-screen.el: Support :var header argument similar to org babel
shell code blocks.

* etc/ORG-NEWS (ob-screen now supports :var header arguments):
Document new feature.
---
 etc/ORG-NEWS      |  9 +++++++++
 lisp/ob-screen.el | 24 +++++++++++++++++++++++-
 2 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index 3cfc2b011d9d..be72cd911f12 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -265,6 +265,15 @@ executable can be configured via =org-ditaa-exec=.
 SVG output can now be generated; note, however, that this requires a
 ditaa version of at least 0.11.0.
 
+*** ob-screen now supports :var header arguments
+
+Example:
+#+BEGIN_src org
+,#+BEGIN_SRC screen :var x=42
+,echo $x
+,#+END_SRC
+#+END_src
+
 ** New and changed options
 
 # Changes dealing with changing default values of customizations,
diff --git a/lisp/ob-screen.el b/lisp/ob-screen.el
index 546c0d5b4342..d2ee65b9681a 100644
--- a/lisp/ob-screen.el
+++ b/lisp/ob-screen.el
@@ -39,6 +39,7 @@
 (org-assert-version)
 
 (require 'ob)
+(require 'ob-shell)
 
 (defvar org-babel-screen-location "screen"
   "The command location for screen.
@@ -54,10 +55,11 @@ In case you want to use a different screen than one selected by your $PATH")
 \"default\" session is used when none is specified in the PARAMS."
   (save-window-excursion
     (let* ((session (cdr (assq :session params)))
+           (var-lines (org-babel-variable-assignments:screen params))
            (socket (org-babel-screen-session-socketname session)))
       (unless socket (org-babel-prep-session:screen session params))
       (org-babel-screen-session-execute-string
-       session (org-babel-expand-body:generic body params)))))
+       session (org-babel-expand-body:generic body params var-lines)))))
 
 (defun org-babel-prep-session:screen (_session params)
   "Prepare SESSION according to the header arguments specified in PARAMS."
@@ -121,6 +123,26 @@ In case you want to use a different screen than one selected by your $PATH")
       (delete-matching-lines "^ +$"))
     tmpfile))
 
+(defun org-babel-variable-assignments:screen (params)
+  "Return variable assignments for a screen source block.
+Dispatches to the appropriate shell-specific assignment function
+based on the :cmd header argument."
+  (let* ((cmd (cdr (assq :cmd params)))
+         (shell (or cmd "sh"))
+         (var-helper (intern-soft
+           (format "org-babel--variable-assignments:%s" shell))))
+    (mapcar ;; Use `mapcar` to apply per-variable formatting because
+     ;; most shell assignment helpers expect one variable at a
+     ;; time. Avoid cleaner `funcall` unless the callee can handle the
+     ;; entire `params` list."
+     (lambda (pair)
+       (let ((varname (symbol-name (car pair)))
+             (val (cdr pair)))
+         (if (and var-helper (fboundp var-helper))
+             (funcall var-helper varname val)
+           (format "%s=%s" varname (org-babel-sh-var-to-sh val)))))
+     (org-babel--get-vars params))))
+
 (defun org-babel-screen-test ()
   "Test if the default setup works.
 The terminal should shortly flicker."
-- 
2.47.3

>From 68e1e84c5706e2f120795cb0f45259bbc76adae6 Mon Sep 17 00:00:00 2001
From: Ken Mankoff <[email protected]>
Date: Wed, 23 Jul 2025 08:45:30 -0400
Subject: [PATCH 2/3] lisp/ob-screen.el: Support custom location for `screen'
 command

* lisp/ob-screen.el: All direct calls to `screen' now use
`org-babel-screen-location'.  Also supports custom screen locations
including paths with spaces.
---
 lisp/ob-screen.el | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/lisp/ob-screen.el b/lisp/ob-screen.el
index 60a473f3978b..546c0d5b4342 100644
--- a/lisp/ob-screen.el
+++ b/lisp/ob-screen.el
@@ -67,8 +67,9 @@ In case you want to use a different screen than one selected by your $PATH")
          (screenrc (cdr (assq :screenrc params)))
          (process-name (concat "org-babel: terminal (" session ")")))
     (apply 'start-process process-name "*Messages*"
-           terminal `("-T" ,(concat "org-babel: " session) "-e" ,org-babel-screen-location
-		      "-c" ,screenrc "-mS" ,session ,cmd))
+           (list terminal "-T" (concat "org-babel: " session) "-e"
+                 org-babel-screen-location "-c" screenrc "-mS" session
+                 cmd))
     ;; XXX: Is there a better way than the following?
     (while (not (org-babel-screen-session-socketname session))
       ;; wait until screen session is available before returning
@@ -83,13 +84,14 @@ In case you want to use a different screen than one selected by your $PATH")
       (let ((tmpfile (org-babel-screen-session-write-temp-file session body)))
         (apply 'start-process (concat "org-babel: screen (" session ")") "*Messages*"
                org-babel-screen-location
-               `("-S" ,socket "-X" "eval" "msgwait 0"
-		 ,(concat "readreg z " tmpfile)
-		 "paste z"))))))
+               (list "-S" socket "-X" "eval" "msgwait 0"
+		     (concat "readreg z " tmpfile)
+		     "paste z"))))))
 
 (defun org-babel-screen-session-socketname (session)
   "Check if SESSION exists by parsing output of \"screen -ls\"."
-  (let* ((screen-ls (shell-command-to-string "screen -ls"))
+  (let* ((screen-cmd (format "%S -ls" org-babel-screen-location))
+         (screen-ls (shell-command-to-string screen-cmd))
          (sockets (delq
 		   nil
                    (mapcar
-- 
2.47.3

Reply via email to