Re: [O] [PATCH] add :session support for ob-js.el

[stardiviner]

Thanks for your refactoring. I must be recklessof this. I will be 
careful next time.



On 03/26/2018 05:09 AM, Nicolas Goaziou wrote:

Hello,

stardiviner  writes:


I attached my new generated patches.


On 03/17/2018 07:14 PM, Nicolas Goaziou wrote:

Some comments follow.

+  (result (if (not (string= (cdr (assq :session params)) "none"))

You can integrate the test above in the `cond':

(cond
   ((string= "none" (cdr (assq :session params))) nil)
   ((string= "*JS REPL*" session) ...)
   ...)

This is modified in new patch.

+ (sit-for .5) (goto-char (point-max))

(sit-for .5)
(goto-char (point-max))

This is in original code, might because ob-js initialize REPL backend
need some time so original author added `site-for`. After test, this
seems does not matter. So remote the sit-for now.

+ (mapc (lambda (var)
+   (insert var) (comint-send-input nil t)
+   (org-babel-comint-wait-for-output session)
+   (sit-for .1) (goto-char (point-max))) var-lines)))
   session))

(dolist (var ...)
 ...)

modified in new patch.

I don't know how to apply master branch new update on my local
`ob-js-session` branch so to get ob-js related commits on top to
generate new patches. (Do you have any idea on this?) The new patches
are behind latest `master` branch. But should be compatible still. If
generating patches based on latest `master` branch commit is must,
I can do that.

I applied you patch. However, I rewrote your commit messages: they need
to refer to modified variables and functions. I also fixed dangling
parenthesis, and fixed a typo in a move from `mapc' + `lambda' to
`dolist'.

Thank you.

Regards,

Re: [O] [PATCH] add :session support for ob-js.el

[Nicolas Goaziou]

Hello,

stardiviner  writes:

> I attached my new generated patches.
>
>
> On 03/17/2018 07:14 PM, Nicolas Goaziou wrote:
>> Some comments follow.
>>> +  (result (if (not (string= (cdr (assq :session params)) "none"))
>> You can integrate the test above in the `cond':
>>
>> (cond
>>   ((string= "none" (cdr (assq :session params))) nil)
>>   ((string= "*JS REPL*" session) ...)
>>   ...)
> This is modified in new patch.
>>> + (sit-for .5) (goto-char (point-max))
>> (sit-for .5)
>> (goto-char (point-max))
> This is in original code, might because ob-js initialize REPL backend
> need some time so original author added `site-for`. After test, this
> seems does not matter. So remote the sit-for now.
>>
>>> + (mapc (lambda (var)
>>> +   (insert var) (comint-send-input nil t)
>>> +   (org-babel-comint-wait-for-output session)
>>> +   (sit-for .1) (goto-char (point-max))) var-lines)))
>>>   session))
>>(dolist (var ...)
>> ...)
> modified in new patch.
>
> I don't know how to apply master branch new update on my local
> `ob-js-session` branch so to get ob-js related commits on top to
> generate new patches. (Do you have any idea on this?) The new patches
> are behind latest `master` branch. But should be compatible still. If
> generating patches based on latest `master` branch commit is must,
> I can do that.

I applied you patch. However, I rewrote your commit messages: they need
to refer to modified variables and functions. I also fixed dangling
parenthesis, and fixed a typo in a move from `mapc' + `lambda' to
`dolist'.

Thank you.

Regards,

-- 
Nicolas Goaziou

Re: [O] [PATCH] add :session support for ob-js.el

[stardiviner]

I attached my new generated patches.


On 03/17/2018 07:14 PM, Nicolas Goaziou wrote:

Some comments follow.

+  (result (if (not (string= (cdr (assq :session params)) "none"))

You can integrate the test above in the `cond':

(cond
  ((string= "none" (cdr (assq :session params))) nil)
  ((string= "*JS REPL*" session) ...)
  ...)

This is modified in new patch.

+ (sit-for .5) (goto-char (point-max))

(sit-for .5)
(goto-char (point-max))
This is in original code, might because ob-js initialize REPL backend 
need some time so original author added `site-for`. After test, this 
seems does not matter. So remote the sit-for now.



+ (mapc (lambda (var)
+   (insert var) (comint-send-input nil t)
+   (org-babel-comint-wait-for-output session)
+   (sit-for .1) (goto-char (point-max))) var-lines)))
  session))

   (dolist (var ...)
...)

modified in new patch.

I don't know how to apply master branch new update on my local 
`ob-js-session` branch so to get ob-js related commits on top to 
generate new patches. (Do you have any idea on this?) The new patches 
are behind latest `master` branch. But should be compatible still. If 
generating patches based on latest `master` branch commit is must, I can 
do that.


>From 140ef4c23fb3e13a772ca01b92b14bccc47043fb Mon Sep 17 00:00:00 2001
From: stardiviner 
Date: Thu, 8 Mar 2018 17:15:58 +0800
Subject: [PATCH 1/4] * ob-js.el: support :session for skewer-mode REPL.

---
 etc/ORG-NEWS  |  4 +++
 lisp/ob-js.el | 81 ---
 2 files changed, 54 insertions(+), 31 deletions(-)

diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index bd9d2a8f7..11ecaa5d3 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -113,6 +113,10 @@ now sort according to the locale’s collation rules instead of by
 code-point.
 
 ** New features
+*** Add ~:session~ support of ob-js for skewer-mode
+#+begin_src js :session "*skewer-repl*"
+console.log("stardiviner")
+#+end_src
 *** Add support for links to LaTeX equations in HTML export
 Use MathJax links when enabled (by ~org-html-with-latex~), otherwise
 add a label to the rendered equation.
diff --git a/lisp/ob-js.el b/lisp/ob-js.el
index 38c8c39ac..920d104f8 100644
--- a/lisp/ob-js.el
+++ b/lisp/ob-js.el
@@ -41,6 +41,8 @@
 (require 'ob)
 
 (declare-function run-mozilla "ext:moz" (arg))
+(declare-function httpd-start "simple-httpd" ())
+(declare-function run-skewer "skewer-mode" ())
 
 (defvar org-babel-default-header-args:js '()
   "Default header arguments for js code blocks.")
@@ -52,7 +54,10 @@
   "Name of command used to evaluate js blocks."
   :group 'org-babel
   :version "24.1"
-  :type 'string)
+  :type '(choice (const "node")
+		 (const "mozrepl")
+		 (const "skewer-mode"))
+  :safe #'stringp)
 
 (defvar org-babel-js-function-wrapper
   "require('sys').print(require('sys').inspect(function(){\n%s\n}()));"
@@ -62,22 +67,13 @@
   "Execute a block of Javascript code with org-babel.
 This function is called by `org-babel-execute-src-block'"
   (let* ((org-babel-js-cmd (or (cdr (assq :cmd params)) org-babel-js-cmd))
+	 (session (cdr (assq :session params)))
  (result-type (cdr (assq :result-type params)))
  (full-body (org-babel-expand-body:generic
 		 body params (org-babel-variable-assignments:js params)))
-	 (result (if (not (string= (cdr (assq :session params)) "none"))
-		 ;; session evaluation
-		 (let ((session (org-babel-prep-session:js
- (cdr (assq :session params)) params)))
-		   (nth 1
-			(org-babel-comint-with-output
-(session (format "%S" org-babel-js-eoe) t body)
-			  (mapc
-			   (lambda (line)
- (insert (org-babel-chomp line))
- (comint-send-input nil t))
-			   (list body (format "%S" org-babel-js-eoe))
-		   ;; external evaluation
+	 (result (cond
+		  ;; no session specified, external evaluation
+		  ((string= session "none")
 		   (let ((script-file (org-babel-temp-file "js-script-")))
 		 (with-temp-file script-file
 		   (insert
@@ -87,7 +83,19 @@ This function is called by `org-babel-execute-src-block'"
 			  full-body)))
 		 (org-babel-eval
 		  (format "%s %s" org-babel-js-cmd
-			  (org-babel-process-file-name script-file)) "")
+			  (org-babel-process-file-name script-file)) "")))
+		  ;; session evaluation
+		  (t
+		   (let ((session (org-babel-prep-session:js
+   (cdr (assq :session params)) params)))
+		 (nth 1
+			  (org-babel-comint-with-output
+			  (session (format "%S" org-babel-js-eoe) t body)
+			(dolist (code (list body (format "%S" org-babel-js-eoe)))
+			  (lambda (code)
+(insert (org-babel-chomp code))
+(comint-send-input nil t)))
+		  )))
 (org-babel-result-cond (cdr (assq :result-params params))
   result (org-babel-js-read result
 
@@ -140,22 +148,33 @@ specifying a variable of the 

Re: [O] [PATCH] add :session support for ob-js.el

[Nicolas Goaziou]

Hello,

stardiviner  writes:

> updated my code. I followed your suggestions. But modify code one by
> one messed code up. So I merge all related commits into one commit.
> Sorry for this.

Some comments follow.
> +(result (if (not (string= (cdr (assq :session params)) "none"))

You can integrate the test above in the `cond':

(cond
 ((string= "none" (cdr (assq :session params))) nil)
 ((string= "*JS REPL*" session) ...)
 ...)
> +   (sit-for .5) (goto-char (point-max))

(sit-for .5)
(goto-char (point-max))

> +   (mapc (lambda (var)
> + (insert var) (comint-send-input nil t)
> + (org-babel-comint-wait-for-output session)
> + (sit-for .1) (goto-char (point-max))) var-lines)))
>  session))

  (dolist (var ...)
   ...)

Also, what's your status wrt FSF papers? This cannot fit as
a TINYCHANGE.

Regard,

-- 
Nicolas Goaziou

Re: [O] [PATCH] add :session support for ob-js.el

[stardiviner]

updated my code. I followed your suggestions. But modify code one by one 
messed code up. So I merge all related commits into one commit. Sorry 
for this.



>From 23ef274fd418c876be85871f8da34738ae15ffd5 Mon Sep 17 00:00:00 2001
From: stardiviner 
Date: Thu, 8 Mar 2018 17:15:58 +0800
Subject: [PATCH] * ob-js.el: support :session for many JS packages.

- ob-js.el: support :session for js-comint.
- ob-js.el: support :session for Indium Node.js REPL.
- ob-js.el: (org-babel-js-initiate-session): merge the `unless' into the `cond'
---
 etc/ORG-NEWS  |   8 
 lisp/ob-js.el | 141 --
 2 files changed, 97 insertions(+), 52 deletions(-)

diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index 10687ba83..5cbc10b8b 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -107,6 +107,14 @@ document, use =shrink= value instead, or in addition to align:
 #+END_EXAMPLE
 
 ** New features
+*** Add support for ob-js :session with Indium
+You can specify :session for js src block with ~*JS REPL*~.
+*** Add support for ob-js :session with js-comint
+After you launched js-comint REPL. You can specify :session for js
+src block with ~*Javascript REPL*~.
+*** Add support for ob-js :session with skewer-mode
+After you launched skewer-mode REPL. You can specify :session for js
+src block with ~*skewer-repl*~.
 *** Add support for open src block in below window
 Set option ~org-src-window-setup~ to ~'split-window-below~.
 *** Add support for links to LaTeX equations in HTML export
diff --git a/lisp/ob-js.el b/lisp/ob-js.el
index 38c8c39ac..93c08c162 100644
--- a/lisp/ob-js.el
+++ b/lisp/ob-js.el
@@ -41,6 +41,9 @@
 (require 'ob)
 
 (declare-function run-mozilla "ext:moz" (arg))
+(declare-function run-js "js-comint" (cmd))
+(declare-function indium-run-node "indium-nodejs" (command))
+(declare-function indium-eval "indium-interaction" (string  callback))
 
 (defvar org-babel-default-header-args:js '()
   "Default header arguments for js code blocks.")
@@ -52,7 +55,11 @@
   "Name of command used to evaluate js blocks."
   :group 'org-babel
   :version "24.1"
-  :type 'string)
+  :type '(choice (const "node")
+		 (const "mozrepl")
+		 (const "js-comint")
+		 (const "skewer-mode"))
+  :safe #'stringp)
 
 (defvar org-babel-js-function-wrapper
   "require('sys').print(require('sys').inspect(function(){\n%s\n}()));"
@@ -62,32 +69,40 @@
   "Execute a block of Javascript code with org-babel.
 This function is called by `org-babel-execute-src-block'"
   (let* ((org-babel-js-cmd (or (cdr (assq :cmd params)) org-babel-js-cmd))
+	   (session (cdr (assq :session params)))
  (result-type (cdr (assq :result-type params)))
  (full-body (org-babel-expand-body:generic
-		 body params (org-babel-variable-assignments:js params)))
-	 (result (if (not (string= (cdr (assq :session params)) "none"))
-		 ;; session evaluation
-		 (let ((session (org-babel-prep-session:js
- (cdr (assq :session params)) params)))
-		   (nth 1
-			(org-babel-comint-with-output
-(session (format "%S" org-babel-js-eoe) t body)
-			  (mapc
-			   (lambda (line)
- (insert (org-babel-chomp line))
- (comint-send-input nil t))
-			   (list body (format "%S" org-babel-js-eoe))
-		   ;; external evaluation
-		   (let ((script-file (org-babel-temp-file "js-script-")))
-		 (with-temp-file script-file
-		   (insert
-			;; return the value or the output
-			(if (string= result-type "value")
-			(format org-babel-js-function-wrapper full-body)
-			  full-body)))
-		 (org-babel-eval
-		  (format "%s %s" org-babel-js-cmd
-			  (org-babel-process-file-name script-file)) "")
+		 body params (org-babel-variable-assignments:js params)))
+	   (result (if (not (string= (cdr (assq :session params)) "none"))
+		 ;; session evaluation
+		 (cond
+		  ;; Indium Node
+		  ((string= "*JS REPL*" session)
+		   (require 'indium-repl)
+		   (unless (get-buffer session)
+			   (indium-run-node))
+		   (indium-eval full-body))
+		  (t
+		   (let ((session (org-babel-prep-session:js
+   (cdr (assq :session params)) params)))
+			   (nth 1
+			(org-babel-comint-with-output
+  (session (format "%S" org-babel-js-eoe) t body)
+(dolist ((code (list body (format "%S" org-babel-js-eoe
+  (lambda (code)
+(insert (org-babel-chomp code))
+(comint-send-input nil t
+		   ;; external evaluation
+		   (let ((script-file (org-babel-temp-file "js-script-")))
+		 (with-temp-file 

Re: [O] [PATCH] add :session support for ob-js.el

[Nicolas Goaziou]

Hello,

stardiviner  writes:

> I added org-mode babel ob-js.el header argument :session.
>
> Following packages :session are supported:
>
> - skewer-mode
>
> - js-comint
>
> - Indium

I don't know any of these, but here come some comments about the code.

>  then create.  Return the initialized session."
>(unless (string= session "none")
>  (cond

Nitpick: You can merge the `unless' into the `cond'.

> - ((string= "*skewer-repl*" session)
> + ((and (string= "js-comint" org-babel-js-cmd) ; `js-comint'
> +(string= "*Javascript REPL*" session))
> +  (require 'js-comint)
> +  (let ((session-buffer "*Javascript REPL*"))
> + (if (and (org-babel-comint-buffer-livep (get-buffer session-buffer))
> +  (comint-check-proc session-buffer))
> + session-buffer
> +   (call-interactively 'run-js)

The `run-js' probably needs to be declared at the beginning of the file.

> +  (cond
> +   ;; Indium Node
> +   ((string= "*JS REPL*" session)
> +(require 'indium-repl)
> +(unless (get-buffer session)
> +  (indium-run-node))

The function above needs to be declared, too.

> +(indium-eval full-body))

So does this one.

> +   (t
> +(let ((session (org-babel-prep-session:js
> +(cdr (assq :session params)) params)))
> +  (nth 1
> +   (org-babel-comint-with-output
> +   (session (format "%S" org-babel-js-eoe) t 
> body)
> + (mapc   ; FIXME: stack on this scope when 
> `skewer-eval'

What does mean this FIXME?

> +  (lambda (line)

Nitpick: `mapc' + `lambda' -> `dolist'

>  (cond
> + ((string= "*skewer-repl*" session)
> +  (require 'skewer-repl)
> +  (let ((session-buffer (get-buffer "*skewer-repl*")))
> + (if (and (org-babel-comint-buffer-livep (get-buffer session-buffer))
> +  (comint-check-proc session-buffer))
> + session-buffer
> +   ;; start skewer REPL.
> +   (sit-for .5)

Why is this `sit-for' needed?

> +   (httpd-start)
> +   (run-skewer)

These functions need to be declared.

Regards,

-- 
Nicolas Goaziou

[O] [PATCH] add :session support for ob-js.el

[stardiviner]

I added org-mode babel ob-js.el header argument :session.

Following packages :session are supported:

- skewer-mode

- js-comint

- Indium

>From 2cdf05cf2fe3e0740997d9861c8ff8f81c163750 Mon Sep 17 00:00:00 2001
From: stardiviner 
Date: Fri, 9 Mar 2018 00:10:54 +0800
Subject: [PATCH 3/3] * ob-js.el: add :session support with js-comint.

---
 lisp/ob-js.el | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/lisp/ob-js.el b/lisp/ob-js.el
index d2fc5e29c..126685813 100644
--- a/lisp/ob-js.el
+++ b/lisp/ob-js.el
@@ -59,6 +59,7 @@
   :version "24.1"
   :type '(choice (const "node")
 		 (const "mozrepl")
+		 (const "js-comint")
 		 (const "skewer-mode"))
   :safe #'stringp)
 
@@ -159,7 +160,17 @@ specifying a variable of the same value."
 then create.  Return the initialized session."
   (unless (string= session "none")
 (cond
- ((string= "*skewer-repl*" session)
+ ((and (string= "js-comint" org-babel-js-cmd) ; `js-comint'
+	   (string= "*Javascript REPL*" session))
+  (require 'js-comint)
+  (let ((session-buffer "*Javascript REPL*"))
+	(if (and (org-babel-comint-buffer-livep (get-buffer session-buffer))
+		 (comint-check-proc session-buffer))
+	session-buffer
+	  (call-interactively 'run-js)
+	  (sit-for .5)
+	  session-buffer)))
+ ((string= "*skewer-repl*" session) ; `skewer-mode'
   (require 'skewer-repl)
   (let ((session-buffer (get-buffer "*skewer-repl*")))
 	(if (and (org-babel-comint-buffer-livep (get-buffer session-buffer))
-- 
2.16.2

>From 4eef9f4fc9534ac45b5d98ba8eab36f764cd7518 Mon Sep 17 00:00:00 2001
From: stardiviner 
Date: Thu, 8 Mar 2018 23:15:16 +0800
Subject: [PATCH 2/3] * ob-js.el: support :session for Indium Node.js REPL.

---
 lisp/ob-js.el | 29 +++--
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/lisp/ob-js.el b/lisp/ob-js.el
index e2a2a9cec..d2fc5e29c 100644
--- a/lisp/ob-js.el
+++ b/lisp/ob-js.el
@@ -70,21 +70,30 @@
   "Execute a block of Javascript code with org-babel.
 This function is called by `org-babel-execute-src-block'"
   (let* ((org-babel-js-cmd (or (cdr (assq :cmd params)) org-babel-js-cmd))
+	 (session (cdr (assq :session params)))
  (result-type (cdr (assq :result-type params)))
  (full-body (org-babel-expand-body:generic
 		 body params (org-babel-variable-assignments:js params)))
 	 (result (if (not (string= (cdr (assq :session params)) "none"))
 		 ;; session evaluation
-		 (let ((session (org-babel-prep-session:js
- (cdr (assq :session params)) params)))
-		   (nth 1
-			(org-babel-comint-with-output
-(session (format "%S" org-babel-js-eoe) t body)
-			  (mapc
-			   (lambda (line)
- (insert (org-babel-chomp line))
- (comint-send-input nil t))
-			   (list body (format "%S" org-babel-js-eoe))
+		 (cond
+		  ;; Indium Node
+		  ((string= "*JS REPL*" session)
+		   (require 'indium-repl)
+		   (unless (get-buffer session)
+			 (indium-run-node))
+		   (indium-eval full-body))
+		  (t
+		   (let ((session (org-babel-prep-session:js
+   (cdr (assq :session params)) params)))
+			 (nth 1
+			  (org-babel-comint-with-output
+  (session (format "%S" org-babel-js-eoe) t body)
+(mapc	; FIXME: stack on this scope when `skewer-eval'
+ (lambda (line)
+   (insert (org-babel-chomp line))
+   (comint-send-input nil t))
+ (list body (format "%S" org-babel-js-eoe
 		   ;; external evaluation
 		   (let ((script-file (org-babel-temp-file "js-script-")))
 		 (with-temp-file script-file
-- 
2.16.2

>From 9c3a57107e4ed598ca24582fa330fe698829b61e Mon Sep 17 00:00:00 2001
From: stardiviner 
Date: Thu, 8 Mar 2018 17:15:58 +0800
Subject: [PATCH] * ob-js.el: support use skewer-mode as session support.

---
 lisp/ob-js.el | 22 +-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/lisp/ob-js.el b/lisp/ob-js.el
index 38c8c39ac..e2a2a9cec 100644
--- a/lisp/ob-js.el
+++ b/lisp/ob-js.el
@@ -27,6 +27,11 @@
 ;;
 ;; This certainly isn't optimally robust, but it seems to be working
 ;; for the basic use cases.
+;;
+;; `skewer-mode' session support:
+;; #+begin_src js :session *skewer-repl*
+;; console.log("chris");
+;; #+end_src
 
 ;;; Requirements:
 
@@ -52,7 +57,10 @@
   "Name of command used to evaluate js blocks."
   :group 'org-babel
   :version "24.1"
-  :type 'string)
+  :type '(choice (const "node")
+		 (const "mozrepl")
+		 (const "skewer-mode"))
+  :safe #'stringp)
 
 (defvar org-babel-js-function-wrapper
   "require('sys').print(require('sys').inspect(function(){\n%s\n}()));"
@@ -142,6 +150,18 @@ specifying a variable of the same value."
 then create.  Return the initialized session."
   (unless (string= session "none")
 (cond
+ ((string= "*skewer-repl*" session)
+  (require 'skewer-repl)
+  (let ((session-buffer (get-buffer