From mboxrd@z Thu Jan  1 00:00:00 1970
From: Nicolas Goaziou <mail@nicolasgoaziou.fr>
Subject: Re: [PATCH] add :session support for ob-js.el
Date: Tue, 13 Mar 2018 09:43:48 +0100
Message-ID: <87efko8iwr.fsf@nicolasgoaziou.fr>
References: <4f31b760-06ac-5c32-8dbd-76f1017d3cd1@gmail.com>
Mime-Version: 1.0
Content-Type: text/plain
Return-path: <emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org>
Received: from eggs.gnu.org ([2001:4830:134:3::10]:37742)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <mail@nicolasgoaziou.fr>) id 1evgBs-0000R7-Gt
	for emacs-orgmode@gnu.org; Tue, 13 Mar 2018 05:25:55 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <mail@nicolasgoaziou.fr>) id 1evgBn-0002pF-5X
	for emacs-orgmode@gnu.org; Tue, 13 Mar 2018 05:25:52 -0400
Received: from relay7-d.mail.gandi.net ([217.70.183.200]:60131)
	by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)
	(Exim 4.71) (envelope-from <mail@nicolasgoaziou.fr>)
	id 1evgBm-0002mT-OH
	for emacs-orgmode@gnu.org; Tue, 13 Mar 2018 05:25:46 -0400
In-Reply-To: <4f31b760-06ac-5c32-8dbd-76f1017d3cd1@gmail.com> (stardiviner's
	message of "Mon, 12 Mar 2018 17:02:26 +0800")
List-Id: "General discussions about Org-mode." <emacs-orgmode.gnu.org>
List-Unsubscribe: <https://lists.gnu.org/mailman/options/emacs-orgmode>,
	<mailto:emacs-orgmode-request@gnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/archive/html/emacs-orgmode/>
List-Post: <mailto:emacs-orgmode@gnu.org>
List-Help: <mailto:emacs-orgmode-request@gnu.org?subject=help>
List-Subscribe: <https://lists.gnu.org/mailman/listinfo/emacs-orgmode>,
	<mailto:emacs-orgmode-request@gnu.org?subject=subscribe>
Errors-To: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org
Sender: "Emacs-orgmode"
	<emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org>
To: stardiviner <numbchild@gmail.com>
Cc: org-mode <emacs-orgmode@gnu.org>

Hello,

stardiviner <numbchild@gmail.com> 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