From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nicolas Goaziou Subject: Re: [PATCH] expose nrepl's timeout setting in ob-clojure.el Date: Wed, 06 Apr 2016 11:43:17 +0200 Message-ID: <87twjfcbt6.fsf@nicolasgoaziou.fr> References: <56FABD8E.2000705@fgiasson.com> Mime-Version: 1.0 Content-Type: text/plain Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:38493) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1anjxY-0001mN-2H for emacs-orgmode@gnu.org; Wed, 06 Apr 2016 05:41:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1anjxU-00012H-3G for emacs-orgmode@gnu.org; Wed, 06 Apr 2016 05:41:11 -0400 Received: from relay3-d.mail.gandi.net ([2001:4b98:c:538::195]:44775) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1anjxT-000121-T4 for emacs-orgmode@gnu.org; Wed, 06 Apr 2016 05:41:08 -0400 In-Reply-To: <56FABD8E.2000705@fgiasson.com> (Frederick Giasson's message of "Tue, 29 Mar 2016 13:38:22 -0400") List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org Sender: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org To: Frederick Giasson Cc: emacs-orgmode@gnu.org 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