Re: [O] [PATCH] expose nrepl's timeout setting in ob-clojure.el

[Nicolas Goaziou]

Hello,

Frederick Giasson  writes:

>>>  (setq result
>>>(nrepl-dict-get
>>> -   (nrepl-sync-request:eval
>>> -expanded (cider-current-connection) (cider-current-session))
>>> +   (let ((nrepl-sync-request-timeout 
>>> org-babel-clojure-sync-nrepl-timeout))
>>> + (nrepl-sync-request:eval
>>> +  expanded (cider-current-connection) (cider-current-session)))
>> You forgot to
>>
>> (defvar nrepl-sync-request-timeout)
>
> This one is defined in the nREPL package. Maybe there is something
> that I don't understand, but do I have to re-defined it here?

The byte-compiler complains if a variable is let-bound but yet not used
in the body. You don't need to define it again but tell the
byte-compiler it is dynamically scoped (using `defvar' without a value).

Regards,

-- 
Nicolas Goaziou

Re: [O] [PATCH] expose nrepl's timeout setting in ob-clojure.el

[Frederick Giasson]

Hi Nicolas,


Some keywords are missing:

   :version "25.1"
   :package-version '(Org . "9.0")

and perhaps

   :safe #'wholenump


Ok good, added.



  (defcustom org-babel-clojure-backend
(cond ((featurep 'cider) 'cider)
(t 'slime))
@@ -94,8 +100,9 @@
 (let ((result-params (cdr (assoc :result-params params
 (setq result
   (nrepl-dict-get
-   (nrepl-sync-request:eval
-expanded (cider-current-connection) (cider-current-session))
+   (let ((nrepl-sync-request-timeout 
org-babel-clojure-sync-nrepl-timeout))
+ (nrepl-sync-request:eval
+  expanded (cider-current-connection) (cider-current-session)))

You forgot to

(defvar nrepl-sync-request-timeout)


This one is defined in the nREPL package. Maybe there is something that 
I don't understand, but do I have to re-defined it here?



I also think it makes sense to merge the 3 patches.


Ok will do.

Thanks,

Fred

Re: [O] [PATCH] expose nrepl's timeout setting in ob-clojure.el

[Nicolas Goaziou]

Hello,

Thank you for the update.

Frederick Giasson  writes:

> +(defcustom org-babel-clojure-sync-nrepl-timeout 10
> +  "Timeout value, in seconds, of a Clojure sync call. 
> +   If the value is nil, timeout is disabled."
> +  :type 'integer
> +  :group 'org-babel)

Some keywords are missing:

  :version "25.1"
  :package-version '(Org . "9.0")

and perhaps

  :safe #'wholenump

>  (defcustom org-babel-clojure-backend
>(cond ((featurep 'cider) 'cider)
>   (t 'slime))
> @@ -94,8 +100,9 @@
> (let ((result-params (cdr (assoc :result-params params
>(setq result
>  (nrepl-dict-get
> - (nrepl-sync-request:eval
> -  expanded (cider-current-connection) (cider-current-session))
> + (let ((nrepl-sync-request-timeout 
> org-babel-clojure-sync-nrepl-timeout))
> +   (nrepl-sync-request:eval
> +expanded (cider-current-connection) (cider-current-session)))

You forgot to

(defvar nrepl-sync-request-timeout)

I also think it makes sense to merge the 3 patches.

Regards,

-- 
Nicolas Goaziou

Re: [O] [PATCH] expose nrepl's timeout setting in ob-clojure.el

[Frederick Giasson]

Hi Nicolas!


-;;; ob-clojure.el --- Babel Functions for Clojure-*- lexical-binding: t; 
-*-
+;;; ob-clojure.el --- org-babel functions for clojure evaluation

Your patch reverts a change introduced in development version. Could you
rebase it on top of latest Org first?


Yeah sorry, I did this from the latest published version, not master. 
Fixed now.





  ;; Copyright (C) 2009-2016 Free Software Foundation, Inc.
  
@@ -55,6 +55,7 @@
  
  (defvar org-babel-default-header-args:clojure '())

  (defvar org-babel-header-args:clojure '((package . :any)))
+(defvar org-babel-clojure-nrepl-timeout 10)

Would it make sense to turn it into a defcustom instead? If so, this
should be added in etc/ORG-NEWS file.


Patch attached.


Bonus points if Worg documentation
about this back-end


Patch attached.




  (defcustom org-babel-clojure-backend
(cond ((featurep 'cider) 'cider)
@@ -67,7 +68,7 @@
  
  (defun org-babel-expand-body:clojure (body params)

"Expand BODY according to PARAMS, return the expanded body."
-  (let* ((vars (org-babel--get-vars params))
+  (let* ((vars (mapcar #'cdr (org-babel-get-header params :var)))

You are also reverting a previous change here.


Fixed.




 (result-params (cdr (assoc :result-params params)))
 (print-level nil) (print-length nil)
 (body (org-babel-trim
@@ -94,8 +95,9 @@
 (let ((result-params (cdr (assoc :result-params params
 (setq result
   (nrepl-dict-get
-   (nrepl-sync-request:eval
-expanded (cider-current-connection) (cider-current-session))
+   (let ((nrepl-sync-request-timeout 
org-babel-clojure-nrepl-timeout))
+ (nrepl-sync-request:eval
+  expanded (cider-current-connection) (cider-current-session)))

It seems you need to define `nrepl-sync-request-timeout' as
a dynamically scoped variable:

   (defvar nrepl-sync-request-timeout)

Indeed "ob-clojure.el" uses lexical binding, even though you disabled it
in the first line of your patch ;)


Fixed :)


Eventually, could you add an appropriate commit message for this patch?
Something like

   ob-clojure: Make REPL timeout configurable

   * lisp/ob-clojure.el (org-babel-clojure-nrepl-timeout):  New variable.
   (org-babel-expand-body:clojure): Use new variable.

   ... explanations about the motivation of the change...


Patch attached.


Hope everything is good now!

Thanks,

Fred

>From c8e89d774590e9b39604252b3a344f95b56e3924 Mon Sep 17 00:00:00 2001
From: Frederick Giasson 
Date: Wed, 6 Apr 2016 10:00:18 -0400
Subject: [PATCH] Document the new option ob-clojure option
 "org-babel-clojure-sync-nrepl-timeout".

---
 org-contrib/babel/languages/ob-doc-clojure.org | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/org-contrib/babel/languages/ob-doc-clojure.org 
b/org-contrib/babel/languages/ob-doc-clojure.org
index 72942df..17c0f20 100644
--- a/org-contrib/babel/languages/ob-doc-clojure.org
+++ b/org-contrib/babel/languages/ob-doc-clojure.org
@@ -90,6 +90,17 @@ Add these lines to your .emacs file to configure CIDER:
   (require 'cider)
 #+END_SRC
 
+Optionally you can specify the Cider timeout by setting the 
=org-babel-clojure-sync-nrepl-timeout=
+setting option. The value is in seconds and if set to =nil= then no timeout 
will occur.
+
+#+BEGIN_SRC emacs-lisp
+  ; Disable Cider's timeout
+  (setq org-babel-clojure-sync-nrepl-timeout nil)
+
+  ; Set the timeout to 20 seconds
+  (setq org-babel-clojure-sync-nrepl-timeout 20)
+#+END_SRC
+
 ** Create a Clojure Project with Leiningen
 
 Create a Leiningen project directory tree:
-- 
1.9.5.msysgit.0

>From 9a81363719f3078d13de9b971440ee6fd456b4b2 Mon Sep 17 00:00:00 2001
From: Frederick Giasson 
Date: Wed, 6 Apr 2016 10:24:54 -0400
Subject: [PATCH 1/2] Update of ORG-NEWS to mention the addition of the new
 setting "org-babel-clojure-sync-nrepl-timeout"

---
 etc/ORG-NEWS | 4 
 1 file changed, 4 insertions(+)

diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index 82d5ad0..e684587 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -106,6 +106,10 @@ becomes
 : ("pdf" . (lambda (file link) (foo)))
 
 ** New features
+*** New option =org-babel-clojure-sync-nrepl-timeout= for =org-babel-clojure=
+This new option tells the =org-babel-clojure= module the length of the timeout 
+of a synchronous call to the nREPL. This value is in seconds. If =nil= then
+no timeout will occur.
 *** New org-protocol key=value syntax
 
 Org-protocol can now handle query-style parameters such as:
-- 
1.9.5.msysgit.0

>From dab6472228d1a0a41831dcae969335d0a7fb7531 Mon Sep 17 00:00:00 2001
From: Frederick Giasson 
Date: Wed, 6 Apr 2016 10:46:35 -0400
Subject: [PATCH 2/2] Addition of a new customization variable called
 "org-babel-clojure-sync-nrepl-timeout" which expose the Cider timeout setting
 as a custom variable.

The timeout is in second, and if nil is specified and timeout is disabled.

With 

Re: [O] [PATCH] expose nrepl's timeout setting in ob-clojure.el

[Nicolas Goaziou]

Hello,

Thank you for the patch.

Frederick Giasson  writes:

> -;;; ob-clojure.el --- Babel Functions for Clojure-*- lexical-binding: t; 
> -*-
> +;;; ob-clojure.el --- org-babel functions for clojure evaluation

Your patch reverts a change introduced in development version. Could you
rebase it on top of latest Org first?

>  
>  ;; Copyright (C) 2009-2016 Free Software Foundation, Inc.
>  
> @@ -55,6 +55,7 @@
>  
>  (defvar org-babel-default-header-args:clojure '())
>  (defvar org-babel-header-args:clojure '((package . :any)))
> +(defvar org-babel-clojure-nrepl-timeout 10)

Would it make sense to turn it into a defcustom instead? If so, this
should be added in etc/ORG-NEWS file. Bonus points if Worg documentation
about this back-end 
>  
>  (defcustom org-babel-clojure-backend
>(cond ((featurep 'cider) 'cider)
> @@ -67,7 +68,7 @@
>  
>  (defun org-babel-expand-body:clojure (body params)
>"Expand BODY according to PARAMS, return the expanded body."
> -  (let* ((vars (org-babel--get-vars params))
> +  (let* ((vars (mapcar #'cdr (org-babel-get-header params :var)))

You are also reverting a previous change here.

>(result-params (cdr (assoc :result-params params)))
>(print-level nil) (print-length nil)
>(body (org-babel-trim
> @@ -94,8 +95,9 @@
> (let ((result-params (cdr (assoc :result-params params
>(setq result
>  (nrepl-dict-get
> - (nrepl-sync-request:eval
> -  expanded (cider-current-connection) (cider-current-session))
> + (let ((nrepl-sync-request-timeout 
> org-babel-clojure-nrepl-timeout))
> +   (nrepl-sync-request:eval
> +expanded (cider-current-connection) (cider-current-session)))

It seems you need to define `nrepl-sync-request-timeout' as
a dynamically scoped variable:

  (defvar nrepl-sync-request-timeout)

Indeed "ob-clojure.el" uses lexical binding, even though you disabled it
in the first line of your patch ;)

Eventually, could you add an appropriate commit message for this patch?
Something like

  ob-clojure: Make REPL timeout configurable

  * lisp/ob-clojure.el (org-babel-clojure-nrepl-timeout):  New variable.
  (org-babel-expand-body:clojure): Use new variable.

  ... explanations about the motivation of the change...


Regards,

-- 
Nicolas Goaziou

[O] [PATCH] expose nrepl's timeout setting in ob-clojure.el

[Frederick Giasson]

Hi everybody,

I am starting to use org-mode to create literate applications in 
Clojure. So far so good, this is a terrific piece of software.


One thing why I wanted to use org-mode is to use it to create Clojure 
Notebooks. The problem I had is that I have many functions that can take 
some time (more than 10 seconds) to complete. This means that I was 
often receiving that error from nrepl:


===
nrepl-send-sync-request: Sync nREPL request timed out (op eval session 
57bdacff-b178-4952-8bf8-5e01ac9d745a code (def umbel 
(create-ontology-structure))

===

The problem is that there is no way to define the nrepl timeout from 
Org-mode. What I did is to expose that nrepl setting in ob-clojure.el. 
This is the ob-clojure.el.diff patch I am proposing here.


The name of the new setting is "org-babel-clojure-nrepl-timeout". If it 
is set to /nil/ then no timeout will occur, otherwise any integer will 
define a timeout value.


This variable can be set from the /.emacs/ global setting file.

It works fine from here, please modify as required.

Thanks,

Fred
diff --git a/lisp/ob-clojure.el b/lisp/ob-clojure.el
index 89a09a9..46031e1 100644
--- a/lisp/ob-clojure.el
+++ b/lisp/ob-clojure.el
@@ -1,4 +1,4 @@
-;;; ob-clojure.el --- Babel Functions for Clojure-*- lexical-binding: t; 
-*-
+;;; ob-clojure.el --- org-babel functions for clojure evaluation
 
 ;; Copyright (C) 2009-2016 Free Software Foundation, Inc.
 
@@ -55,6 +55,7 @@
 
 (defvar org-babel-default-header-args:clojure '())
 (defvar org-babel-header-args:clojure '((package . :any)))
+(defvar org-babel-clojure-nrepl-timeout 10)
 
 (defcustom org-babel-clojure-backend
   (cond ((featurep 'cider) 'cider)
@@ -67,7 +68,7 @@
 
 (defun org-babel-expand-body:clojure (body params)
   "Expand BODY according to PARAMS, return the expanded body."
-  (let* ((vars (org-babel--get-vars params))
+  (let* ((vars (mapcar #'cdr (org-babel-get-header params :var)))
 (result-params (cdr (assoc :result-params params)))
 (print-level nil) (print-length nil)
 (body (org-babel-trim
@@ -94,8 +95,9 @@
(let ((result-params (cdr (assoc :result-params params
 (setq result
   (nrepl-dict-get
-   (nrepl-sync-request:eval
-expanded (cider-current-connection) (cider-current-session))
+   (let ((nrepl-sync-request-timeout 
org-babel-clojure-nrepl-timeout))
+ (nrepl-sync-request:eval
+  expanded (cider-current-connection) (cider-current-session)))
(if (or (member "output" result-params)
(member "pp" result-params))
"out"

warning: LF will be replaced by CRLF in lisp/ob-clojure.el.
The file will have its original line endings in your working directory.