emacs-orgmode
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Frederick Giasson
Subject: Re: [O] [PATCH] expose nrepl's timeout setting in ob-clojure.el
Date: Wed, 6 Apr 2016 10:48:51 -0400
User-agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.7.1

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

Attachment: 0001-Document-the-new-option-ob-clojure-option-org-babel-.patch
Description: Text document

Attachment: 0001-Update-of-ORG-NEWS-to-mention-the-addition-of-the-ne.patch
Description: Text document

Attachment: 0002-Addition-of-a-new-customization-variable-called-org-.patch
Description: Text document


reply via email to

[Prev in Thread] Current Thread [Next in Thread]