Hi Ihor, On 2025-11-15 at 05:28 -08, Ihor Radchenko <[email protected]> wrote... >> 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 > > There is no need to duplicate the changelog entry.
I'm not sure what you mean by this. >> 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. > > A minor comment: New function should probably appear in the changelog. Added. >> +(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." > > The docstring should mention all the function arguments. > See D.6 Tips for Documentation Strings in Elisp manual. > Also, M-x checkdoc. Fixed. >> + (let* ((cmd (cdr (assq :cmd params))) >> + (shell (or cmd "sh")) > > In other places, there is no fallback shell defined. Also, why "sh" and > not `shell-file-name'? Fixed. >> + (var-helper (intern-soft >> + (format "org-babel--variable-assignments:%s" shell)))) > > Why not simply calling `org-babel-variable-assignments:shell'? I think this is because of non-standard shells like fish, but I am not sure how I came to this code at this point. It's been a few years. There has been some LLM use in patch #3. If that is not OK, please let me know. I hope #1 and #2 are OK even if not #3. -k.
>From 8920ac06df5cdafdd2c8da9ab7da6ceb40015ecf 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 37feda8f6bbf..dca531bc201a 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 b231ed99c026d1ca3ab6eecc0d859a1c3b879d75 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 dca531bc201a..bd0af9dffd5f 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
>From 1e3ef701fbfd3d332f89dc086846e48d66da56bc 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 | 13 +++++++++++++ lisp/ob-screen.el | 26 +++++++++++++++++++++++++- 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS index f2347a401f80..ac1b61be5509 100644 --- a/etc/ORG-NEWS +++ b/etc/ORG-NEWS @@ -276,6 +276,15 @@ Runtime error near line 2: attempt to write a readonly database (8) [ Babel evaluation exited with code 1 ] #+end_example +*** 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, @@ -545,6 +554,10 @@ The new function constructs an invisibility spec without folds and ellipses, suitable for ~org-string-width~. This can be helpful for performance if ~org-string-width~ is called multiple times. +*** New function ~org-babel-variable-assignments:screen~ + +The new function returns variable assignments for a screen source block. + *** New command ~org-link-preview~ to preview Org links This command replaces ~org-toggle-inline-images~, which is now diff --git a/lisp/ob-screen.el b/lisp/ob-screen.el index bd0af9dffd5f..dce683534ac6 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,28 @@ 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 assignment strings for a screen source block. + +PARAMS is the header argument alist. Dispatches to a +shell-specific helper of the form +`org-babel--variable-assignments:CMD' based on the :cmd header +argument, falling back to a generic NAME=VALUE assignment when no +such helper is found or bound." + (let* ((cmd (cdr (assq :cmd params))) + (helper-sym (and cmd + (intern-soft + (format "org-babel--variable-assignments:%s" cmd)))) + (helper (and (fboundp helper-sym) helper-sym))) + (mapcar + (lambda (pair) + (let ((name (symbol-name (car pair))) + (val (cdr pair))) + (if helper + (funcall helper name val) + (format "%s=%s" name (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
