Patch: send 'return' after babel blocks sent to ob-screen [9.3.7 (9.3.7-12-g5d6868-elpaplus @ /home/kdm/.emacs.d/elpa/org-plus-contrib-20200706/)]

Patch: send 'return' after babel blocks sent to ob-screen [9.3.7 (9.3.7-12-g5d6868-elpaplus @ /home/kdm/.emacs.d/elpa/org-plus-contrib-20200706/)]

[Ken Mankoff]

Using ob-screen and this example:

#+begin_src screen
ls
#+end_src

The "ls" is transmitted to the screen session, but without a newline, it does not execute. The following patch adds a newline to all babel blocks sent to the screen session.

diff --git a/lisp/ob-screen.el b/lisp/ob-screen.el
index 75a2dc691..5616143fe 100644
--- a/lisp/ob-screen.el
+++ b/lisp/ob-screen.el
@@ -108,7 +108,7 @@ In case you want to use a different screen than one selected by your $PATH")
   (let ((tmpfile (org-babel-temp-file "screen-")))
     (with-temp-file tmpfile
       (insert body)
-
+      (insert "\n")
       ;; org-babel has superfluous spaces
       (goto-char (point-min))
       (delete-matching-lines "^ +$"))


Also, can anyone clarify why there is both

./contrib/lisp/org-screen.el
./lisp/ob-screen.el

?

Thanks,

  -k.

Re: Patch: send 'return' after babel blocks sent to ob-screen [9.3.7 (9.3.7-12-g5d6868-elpaplus @ /home/kdm/.emacs.d/elpa/org-plus-contrib-20200706/)]

[Kyle Meyer]

Ken Mankoff writes:

> Using ob-screen and this example:
>
> #+begin_src screen
> ls
> #+end_src
>
> The "ls" is transmitted to the screen session, but without a newline,
> it does not execute.

I see that on my end too.  I've never used ob-screen, and, with this
sort of issue, the same might be true for nearly everybody, assuming
this wasn't a recent regression.  (I don't spot any obvious culprits.)

> The following patch adds a newline to all babel blocks sent to the screen session.
>
> diff --git a/lisp/ob-screen.el b/lisp/ob-screen.el
> index 75a2dc691..5616143fe 100644
> --- a/lisp/ob-screen.el
> +++ b/lisp/ob-screen.el
> @@ -108,7 +108,7 @@ In case you want to use a different screen than one selected by your $PATH")
>    (let ((tmpfile (org-babel-temp-file "screen-")))
>      (with-temp-file tmpfile
>        (insert body)
> -
> +      (insert "\n")
>        ;; org-babel has superfluous spaces
>        (goto-char (point-min))
>        (delete-matching-lines "^ +$"))

Thanks.  Looks good and seems to work on my end.  Could prepare that as
a patch with a commit message (details at
https://orgmode.org/worg/org-contribute.html)?

[PATCH] 3 improvements to ob-screen

[Ken Mankoff]

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

Hello,

The attached three patches implement a bug fix, a simplification, and a new feature to ob-screen.

bug fix: 001 Execute last line in block (send newline)
simplify: 002 Don't add extra characters to session name
feature: 003 Accept :screenrc header argument

I've signed the FSF docs but do not see my name here: https://orgmode.org/worg/org-contribute.html under "Current contributors".

  -k.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-ob-screen-Execute-last-line-in-block-send-newline.patch --]
[-- Type: text/x-diff, Size: 734 bytes --]

From cb014cd2da79b30967dd24fba37d7f75569ed92d Mon Sep 17 00:00:00 2001
From: "Kenneth D. Mankoff" <mankoff@gmail.com>
Date: Tue, 14 Jul 2020 13:29:36 -0700
Subject: [PATCH 1/3] ob-screen: Execute last line in block (send newline)

---
 lisp/ob-screen.el | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lisp/ob-screen.el b/lisp/ob-screen.el
index 75a2dc691..3edc2c265 100644
--- a/lisp/ob-screen.el
+++ b/lisp/ob-screen.el
@@ -108,6 +108,7 @@ In case you want to use a different screen than one selected by your $PATH")
   (let ((tmpfile (org-babel-temp-file "screen-")))
     (with-temp-file tmpfile
       (insert body)
+      (insert "\n")
 
       ;; org-babel has superfluous spaces
       (goto-char (point-min))
-- 
2.25.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-ob-screen-Respect-session-name.-Don-t-prepend-org-ba.patch --]
[-- Type: text/x-diff, Size: 2185 bytes --]

From e6ee8bff913da35a0183f8e2c1db7021342492d9 Mon Sep 17 00:00:00 2001
From: "Kenneth D. Mankoff" <mankoff@gmail.com>
Date: Tue, 14 Jul 2020 13:41:35 -0700
Subject: [PATCH 2/3] ob-screen: Respect :session name. Don't prepend
 'org-babel-session-'

---
 etc/ORG-NEWS      | 7 +++++++
 lisp/ob-screen.el | 6 ++----
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index c366f61e0..623fa9dc3 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -380,6 +380,13 @@ From ~org-enable-priority-commands~ to ~org-priority-enable-commands~.
 From ~org-show-priority~ to ~org-priority-show~.
 
 ** Miscellaneous
+*** =ob-screen.el=: Respect screen =:session= name
+
+Screen babel session are now named based on the =:session= header 
+argument (defaults to ~default~). 
+
+Previously all session names had  ~org-babel-session-~ prepended.
+
 *** Forward/backward paragraph functions in line with the rest of Emacs
 ~org-forward-paragraph~ and ~org-backward-paragraph~, bound to
 ~<C-UP>~ and ~<C-DOWN>~ functions mimic more closely behaviour of
diff --git a/lisp/ob-screen.el b/lisp/ob-screen.el
index 3edc2c265..fe4698203 100644
--- a/lisp/ob-screen.el
+++ b/lisp/ob-screen.el
@@ -62,8 +62,7 @@ In case you want to use a different screen than one selected by your $PATH")
          (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" "/dev/null" "-mS" ,(concat "org-babel-session-" session)
-		      ,cmd))
+		      "-c" "/dev/null" "-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
@@ -97,8 +96,7 @@ In case you want to use a different screen than one selected by your $PATH")
 			 nil
 			 (mapcar
 			  (lambda (x)
-			    (when (string-match
-				   (concat "org-babel-session-" session) x)
+			    (when (string-match x)
 			      x))
 			  sockets)))))
     (when match-socket (car (split-string match-socket)))))
-- 
2.25.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0003-ob-screen-accept-screenrc-header-argument.patch --]
[-- Type: text/x-diff, Size: 2394 bytes --]

From 866f3c4788e58bf26dcf9c90cb7638f48a6a4ecc Mon Sep 17 00:00:00 2001
From: "Kenneth D. Mankoff" <mankoff@gmail.com>
Date: Tue, 14 Jul 2020 13:48:52 -0700
Subject: [PATCH 3/3] ob-screen: accept :screenrc header argument

---
 etc/ORG-NEWS      | 6 ++++++
 lisp/ob-screen.el | 5 +++--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index 623fa9dc3..5fb2af22e 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -229,6 +229,12 @@ directory configured for ClojureScript will /not/ work.
 Babel Java blocks recognize header argument =:cmdargs= and pass its
 value in call to =java=.
 
+*** =ob-screen.el=: Screen now accepts =:screenrc= header argument
+
+Screen blocks now recognize the =:screenrc= header argument and pass
+its value to the screen command via the "-c" option. The default
+remains =/dev/null= (i.e. a clean screen session)
+
 *** =RET= and =C-j= now obey ~electric-indent-mode~
 
 Since Emacs 24.4, ~electric-indent-mode~ is enabled by default.  In
diff --git a/lisp/ob-screen.el b/lisp/ob-screen.el
index fe4698203..7115080f8 100644
--- a/lisp/ob-screen.el
+++ b/lisp/ob-screen.el
@@ -40,7 +40,7 @@
 In case you want to use a different screen than one selected by your $PATH")
 
 (defvar org-babel-default-header-args:screen
-  '((:results . "silent") (:session . "default") (:cmd . "sh") (:terminal . "xterm"))
+  '((:results . "silent") (:session . "default") (:cmd . "sh") (:terminal . "xterm") (:screenrc . "/dev/null"))
   "Default arguments to use when running screen source blocks.")
 
 (defun org-babel-execute:screen (body params)
@@ -59,10 +59,11 @@ In case you want to use a different screen than one selected by your $PATH")
   (let* ((session (cdr (assq :session params)))
          (cmd (cdr (assq :cmd params)))
          (terminal (cdr (assq :terminal params)))
+         (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" "/dev/null" "-mS" ,session ,cmd))
+		      "-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
-- 
2.25.1

Re: [PATCH] 3 improvements to ob-screen

[Kyle Meyer]

Ken Mankoff writes:

> I've signed the FSF docs but do not see my name here:
> https://orgmode.org/worg/org-contribute.html under "Current
> contributors".

Thanks for the information.  I'll add you to the list.

> Subject: [PATCH 1/3] ob-screen: Execute last line in block (send newline)

Could you add a changelog entry to the body of this commit message and
the others?  As an example, here's the one in the most recent commit on
master:

    * lisp/ox.el (org-export-smart-quotes-alist): Add Romanian variants for
      smart quotes.

> Subject: [PATCH 3/3] ob-screen: accept :screenrc header argument
>
> ---
>  etc/ORG-NEWS      | 6 ++++++
>  lisp/ob-screen.el | 5 +++--
>  2 files changed, 9 insertions(+), 2 deletions(-)

I didn't comment on this one elsewhere.  Looks good to me.

> diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
> index 623fa9dc3..5fb2af22e 100644
[...]
> diff --git a/lisp/ob-screen.el b/lisp/ob-screen.el
> index fe4698203..7115080f8 100644
> --- a/lisp/ob-screen.el
> +++ b/lisp/ob-screen.el
> @@ -40,7 +40,7 @@
>  In case you want to use a different screen than one selected by your $PATH")
>  
>  (defvar org-babel-default-header-args:screen
> -  '((:results . "silent") (:session . "default") (:cmd . "sh") (:terminal . "xterm"))
> +  '((:results . "silent") (:session . "default") (:cmd . "sh") (:terminal . "xterm") (:screenrc . "/dev/null"))
>    "Default arguments to use when running screen source blocks.")

nit-pick: Please move the new entry to the next line.

Re: [PATCH] 3 improvements to ob-screen

[Ken Mankoff]

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


On 2020-07-14 at 20:08 -07, Kyle Meyer <kyle@kyleam.com> wrote...
> Could you add a changelog entry to the body of this commit message and
> the others?

I made the changes you requested. Updated patches attached.

  -k.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-ob-screen-Execute-last-line-in-block-send-newline.patch --]
[-- Type: text/x-diff, Size: 825 bytes --]

From d7b7555969ce0e58653c5b5a78d9a4a0bebbacdf Mon Sep 17 00:00:00 2001
From: "Kenneth D. Mankoff" <mankoff@gmail.com>
Date: Tue, 14 Jul 2020 13:29:36 -0700
Subject: [PATCH 1/3] ob-screen: Execute last line in block (send newline)

* lisp/ob-screen.el (org-babel-screen-session-write-temp-file): insert
newline after body.
---
 lisp/ob-screen.el | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lisp/ob-screen.el b/lisp/ob-screen.el
index 75a2dc691..3edc2c265 100644
--- a/lisp/ob-screen.el
+++ b/lisp/ob-screen.el
@@ -108,6 +108,7 @@ In case you want to use a different screen than one selected by your $PATH")
   (let ((tmpfile (org-babel-temp-file "screen-")))
     (with-temp-file tmpfile
       (insert body)
+      (insert "\n")
 
       ;; org-babel has superfluous spaces
       (goto-char (point-min))
-- 
2.25.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-ob-screen-Respect-session-name.-Don-t-prepend-org-ba.patch --]
[-- Type: text/x-diff, Size: 2486 bytes --]

From 05dcb2a9f9acb6d15c88ac67d9268243b620cf5e Mon Sep 17 00:00:00 2001
From: "Kenneth D. Mankoff" <mankoff@gmail.com>
Date: Tue, 14 Jul 2020 13:41:35 -0700
Subject: [PATCH 2/3] ob-screen: Respect :session name. Don't prepend
 'org-babel-session-'

* lisp/ob-screen.el (org-babel-prep-session:screen): Remove concat of
"org-babel-session-" string onto session name when creating session.

* lisp/ob-screen.el (org-babel-screen-session-socketname): Remove
concat of "org-babel-session-" string onto session name searching for
existing screen session.
---
 etc/ORG-NEWS      | 7 +++++++
 lisp/ob-screen.el | 6 ++----
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index c366f61e0..623fa9dc3 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -380,6 +380,13 @@ From ~org-enable-priority-commands~ to ~org-priority-enable-commands~.
 From ~org-show-priority~ to ~org-priority-show~.
 
 ** Miscellaneous
+*** =ob-screen.el=: Respect screen =:session= name
+
+Screen babel session are now named based on the =:session= header 
+argument (defaults to ~default~). 
+
+Previously all session names had  ~org-babel-session-~ prepended.
+
 *** Forward/backward paragraph functions in line with the rest of Emacs
 ~org-forward-paragraph~ and ~org-backward-paragraph~, bound to
 ~<C-UP>~ and ~<C-DOWN>~ functions mimic more closely behaviour of
diff --git a/lisp/ob-screen.el b/lisp/ob-screen.el
index 3edc2c265..fe4698203 100644
--- a/lisp/ob-screen.el
+++ b/lisp/ob-screen.el
@@ -62,8 +62,7 @@ In case you want to use a different screen than one selected by your $PATH")
          (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" "/dev/null" "-mS" ,(concat "org-babel-session-" session)
-		      ,cmd))
+		      "-c" "/dev/null" "-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
@@ -97,8 +96,7 @@ In case you want to use a different screen than one selected by your $PATH")
 			 nil
 			 (mapcar
 			  (lambda (x)
-			    (when (string-match
-				   (concat "org-babel-session-" session) x)
+			    (when (string-match x)
 			      x))
 			  sockets)))))
     (when match-socket (car (split-string match-socket)))))
-- 
2.25.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0003-ob-screen-accept-screenrc-header-argument.patch --]
[-- Type: text/x-diff, Size: 2647 bytes --]

From d99c637f1cade54ab2f66b72ec32026a15aa03de Mon Sep 17 00:00:00 2001
From: "Kenneth D. Mankoff" <mankoff@gmail.com>
Date: Tue, 14 Jul 2020 13:48:52 -0700
Subject: [PATCH 3/3] ob-screen: accept :screenrc header argument

* lisp/ob-screen.el (org-babel-default-header-args:screen): Add
default header argument `:screenrc' and value "/dev/null".

* lisp/ob-screen.el (org-babel-prep-session:screen): Use header
argument in variable `screenrc' and not hard-coded value.
---
 etc/ORG-NEWS      | 6 ++++++
 lisp/ob-screen.el | 6 ++++--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index 623fa9dc3..5fb2af22e 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -229,6 +229,12 @@ directory configured for ClojureScript will /not/ work.
 Babel Java blocks recognize header argument =:cmdargs= and pass its
 value in call to =java=.
 
+*** =ob-screen.el=: Screen now accepts =:screenrc= header argument
+
+Screen blocks now recognize the =:screenrc= header argument and pass
+its value to the screen command via the "-c" option. The default
+remains =/dev/null= (i.e. a clean screen session)
+
 *** =RET= and =C-j= now obey ~electric-indent-mode~
 
 Since Emacs 24.4, ~electric-indent-mode~ is enabled by default.  In
diff --git a/lisp/ob-screen.el b/lisp/ob-screen.el
index fe4698203..021fef60d 100644
--- a/lisp/ob-screen.el
+++ b/lisp/ob-screen.el
@@ -40,7 +40,8 @@
 In case you want to use a different screen than one selected by your $PATH")
 
 (defvar org-babel-default-header-args:screen
-  '((:results . "silent") (:session . "default") (:cmd . "sh") (:terminal . "xterm"))
+  '((:results . "silent") (:session . "default") (:cmd . "sh")
+    (:terminal . "xterm") (:screenrc . "/dev/null"))
   "Default arguments to use when running screen source blocks.")
 
 (defun org-babel-execute:screen (body params)
@@ -59,10 +60,11 @@ In case you want to use a different screen than one selected by your $PATH")
   (let* ((session (cdr (assq :session params)))
          (cmd (cdr (assq :cmd params)))
          (terminal (cdr (assq :terminal params)))
+         (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" "/dev/null" "-mS" ,session ,cmd))
+		      "-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
-- 
2.25.1

Re: [PATCH] 3 improvements to ob-screen

[Kyle Meyer]

Ken Mankoff writes:

> On 2020-07-14 at 20:08 -07, Kyle Meyer <kyle@kyleam.com> wrote...
>> Could you add a changelog entry to the body of this commit message and
>> the others?
>
> I made the changes you requested. Updated patches attached.

Thank you.  Applied, with a few minor tweaks to the commit messages and
deleting trailing white space in the NEWS entry of patch 2.

Re: [PATCH] 3 improvements to ob-screen

[Kyle Meyer]

Kyle Meyer writes:

> Ken Mankoff writes:
>
>> On 2020-07-14 at 20:08 -07, Kyle Meyer <kyle@kyleam.com> wrote...
>>> Could you add a changelog entry to the body of this commit message and
>>> the others?
>>
>> I made the changes you requested. Updated patches attached.
>
> Thank you.  Applied, with a few minor tweaks to the commit messages and
> deleting trailing white space in the NEWS entry of patch 2.

Well, applied, but not pushed yet.  In the second patch, the session
name is no longer passed to string-match; it was in the original patch
at <https://orgmode.org/list/874kqgsm1o.fsf@gmail.com>.  I'll fix it up
locally before pushing.

Re: [PATCH] 3 improvements to ob-screen

[Ken Mankoff]

On 2020-07-15 at 20:12 -07, Kyle Meyer <kyle@kyleam.com> wrote...
> Well, applied, but not pushed yet. In the second patch, the session
> name is no longer passed to string-match; it was in the original patch
> at <https://orgmode.org/list/874kqgsm1o.fsf@gmail.com>. I'll fix it up
> locally before pushing.

Argh - I apologize for introducing that bug. Thank you for catching it and accepting the patches. I'll look at the tweaks you made to the commit messages to learn what is preferred for my next contribution.

  -k.