Ihor Radchenko <[email protected]> writes:
> Simon Cossar <[email protected]> writes:
>
>> I'll try creating a patch.
>
> Thanks for the patch!
> Would you be interested to fix some other remaining issues in the
> `org-babel-expand-body:clojure'? There are still things like
>
> (eq org-babel-clojure-backend 'cider)
>
> that do not look like they should belong there.
Sure, I'll get to that next.
>
> As for the tests, the first test actually depends on clojure executable,
> although indirectly - the value of `org-babel-clojure-backend' depends
> on what is installed on the system:
>
> (defcustom org-babel-clojure-backend (cond
> ((executable-find "bb") 'babashka)
> ((executable-find "clojure")
> 'clojure-cli)
> ((featurep 'cider) 'cider)
> ((featurep 'inf-clojure)
> 'inf-clojure)
> ((featurep 'slime) 'slime)
> (t nil))
>
> To run babel tests from command line, you need to explicitly activate
> them. Many babel tests are disabled by default.
> You need to set several variables for make (either from command line or
> in local.mk). For example, here is what I use:
>
> BTEST_POST = -L ~/.emacs.d/straight/repos/geiser/elisp -L
> ~/.emacs.d/straight/repos/guile -L ~/.emacs.d/straight/repos/inf-ruby
> BTEST_OB_LANGUAGES = awk C fortran maxima lilypond octave perl
> python java sqlite eshell calc lua R scheme ruby plantuml
Thanks! That got me pointed in the right direction. From the command
line I'm running:
emacs -batch \
-L ~/elisp/org-mode/lisp/ \
-L ~/elisp/org-mode/testing/ \
-l ob-clojure \
-l ert \
-l ~/elisp/org-mode/testing/lisp/test-ob-clojure.el \
-f ert-run-tests-batch-and-exit
>> Subject: [PATCH 1/2] lisp/ob-clojure.el: Stop tangling code that is used for
>> evaluation
>
> Nitpick: `ob-clojure--prepare-for-evaluation' (quote).
>
>> - ;; If the result param is set to "output" we don't have to do
>> - ;; anything special and just let the backend handle everything
>> - (if (member "output" result-params)
>> - body
>> -
>> - ;; If the result is not "output" (i.e. it's "value"), disable
>> - ;; stdout output and print the last returned value. Use pprint
>> - ;; instead of prn when results param is "pp" or "code".
>
> You removed these comments in the patch. Is there any rationale? Or just
> an omission.
That was just an omission. I've re-added the comments. I changed "it's"
to "it is". For some reason I thought that was clearer, but on second
thought I'm not sure it makes much difference.
>
>> ;;; Code:
>> +(require 'org-test "../testing/org-test")
>
> Looks like your patch is not against the latest main.
> We already have this line in this file on the latest main.
>
>> +;; execute
>> +(ert-deftest ob-clojure/org-babel-execute ()
>> + "org-babel-execute:clojure correctly handles :result-params"
>> + (skip-unless (executable-find "clojure"))
Yes, I see that was added in commit ff9df1430. That's fixed now.
>
> This hard-codes the test only for clojure.
> An alternative would be checking for the value of
> `org-babel-clojure-backend' (automatically set).
>
>> + (let ((org-babel-clojure-backend 'clojure-cli))
>
> Same problem here. Why not using the default backend (if any)?
That makes sense. Since both tests depend on the Clojure executable,
maybe it's safe enough to just allow the user-error that's called in
org-babel-execute:clojure and org-babel-expand-body:clojure to handle
the case of no executable being configured. That simplifies the
ob-clojure/org-babel-execute test to:
(ert-deftest ob-clojure/org-babel-execute ()
"org-babel-execute:clojure correctly handles :result-params."
(should (equal 3
(org-babel-execute:clojure
"(+ 1 2)"
'((:result-params "value")))))
(should (equal ""
(org-babel-execute:clojure
"(+ 1 2)"
'((:result-params "output"))))))
--
Simon Cossar
diff --git c/lisp/ob-clojure.el w/lisp/ob-clojure.el
index 8f07d3d3f..fa1aaced3 100644
--- c/lisp/ob-clojure.el
+++ w/lisp/ob-clojure.el
@@ -133,9 +133,8 @@
:group 'org-babel
:package-version '(Org . "9.7"))
-(defun org-babel-expand-body:clojure (body params &optional cljs-p)
- "Expand BODY according to PARAMS, return the expanded body.
-When CLJS-P is non-nil, expand in a cljs context instead of clj."
+(defun org-babel-expand-body:clojure (body params &optional _cljs-p)
+ "Expand BODY according to PARAMS, return the expanded body."
(let* ((vars (org-babel--get-vars params))
(backend-override (cdr (assq :backend params)))
(org-babel-clojure-backend
@@ -144,51 +143,30 @@ When CLJS-P is non-nil, expand in a cljs context instead of clj."
(org-babel-clojure-backend org-babel-clojure-backend)
(t (user-error "You need to customize `org-babel-clojure-backend'
or set the `:backend' header argument"))))
- (ns (or (cdr (assq :ns params))
- (if (eq org-babel-clojure-backend 'cider)
- (or cider-buffer-ns
- (let ((repl-buf (cider-current-connection)))
- (and repl-buf (buffer-local-value
- 'cider-buffer-ns repl-buf))))
- org-babel-clojure-default-ns)))
- (result-params (cdr (assq :result-params params)))
- (print-level nil)
- (print-length nil)
- (body (org-trim
- (concat
- ;; Source block specified namespace :ns.
- (and (cdr (assq :ns params)) (format "(ns %s)\n" ns))
- ;; Variables binding.
- (if (null vars) (org-trim body)
- ;; Remove comments, they break (let [...] ...) bindings
- (let ((body (replace-regexp-in-string "^[ ]*;+.*$" "" body)))
- (format "(let [%s]\n%s)"
- (mapconcat
- (lambda (var)
- (format "%S '%S" (car var) (cdr var)))
- vars
- "\n ")
- body)))))))
- ;; If the result param is set to "output" we don't have to do
- ;; anything special and just let the backend handle everything
- (if (member "output" result-params)
- body
-
- ;; If the result is not "output" (i.e. it's "value"), disable
- ;; stdout output and print the last returned value. Use pprint
- ;; instead of prn when results param is "pp" or "code".
- (concat
- (if (or (member "code" result-params)
- (member "pp" result-params))
- (concat (if cljs-p
- "(require '[cljs.pprint :refer [pprint]])"
- "(require '[clojure.pprint :refer [pprint]])")
- " (pprint ")
- "(prn ")
- (if cljs-p
- "(binding [cljs.core/*print-fn* (constantly nil)]"
- "(binding [*out* (java.io.StringWriter.)]")
- body "))"))))
+ (ns (or (cdr (assq :ns params))
+ (if (eq org-babel-clojure-backend 'cider)
+ (or cider-buffer-ns
+ (let ((repl-buf (cider-current-connection)))
+ (and repl-buf (buffer-local-value
+ 'cider-buffer-ns repl-buf))))
+ org-babel-clojure-default-ns)))
+ (print-level nil)
+ (print-length nil))
+ (org-trim
+ (concat
+ ;; Source block specified namespace :ns.
+ (and (cdr (assq :ns params)) (format "(ns %s)\n" ns))
+ ;; Variables binding.
+ (if (null vars) (org-trim body)
+ ;; Remove comments, they break (let [...] ...) bindings
+ (let ((body (replace-regexp-in-string "^[ ]*;+.*$" "" body)))
+ (format "(let [%s]\n%s)"
+ (mapconcat
+ (lambda (var)
+ (format "%S '%S" (car var) (cdr var)))
+ vars
+ "\n ")
+ body)))))))
(defvar ob-clojure-inf-clojure-filter-out)
(defvar ob-clojure-inf-clojure-tmp-output)
@@ -300,6 +278,30 @@ The PARAMS from Babel are not used in this function."
(format "%s %s" cmd (org-babel-process-file-name script-file))
"")))
+(defun ob-clojure--prepare-for-evaluation (expanded-body params cljs-p)
+ "Wrap EXPANDED-BODY in a print expression, depending on PARAMS and CLJS-P."
+ (let* ((result-params (cdr (assq :result-params params)))
+ ;; If the result param is set to "output", we don't have to do
+ ;; anything special and just let the backend handle everything.
+ (output (if (member "output" result-params)
+ expanded-body
+ ;; If the result param is not "output" (i.e. it is "value"),
+ ;; disable stdout output and print the last returned value.
+ ;; Use pprint instead of prn when result param is "pp" or "code".
+ (concat
+ (if (or (member "code" result-params)
+ (member "pp" result-params))
+ (concat (if cljs-p
+ "(require '[cljs.pprint :refer [pprint]])"
+ "(require '[clojure.pprint :refer [pprint]])")
+ " (pprint")
+ "(prn ")
+ (if cljs-p
+ "(binding [cljs.core/*print-fn* (constantly nil)])"
+ "(binding [*out* (java.io.StringWriter.)]")
+ expanded-body "))"))))
+ output))
+
(defun org-babel-execute:clojure (body params &optional cljs-p)
"Execute the BODY block of Clojure code with PARAMS using Babel.
When CLJS-P is non-nil, execute with a ClojureScript backend
@@ -321,23 +323,24 @@ or set the `:backend' header argument"
;; ClojureScript syntax when we either evaluate a
;; ClojureScript source block or use the nbb backend.
(cljs-p (or cljs-p (eq org-babel-clojure-backend 'nbb))))
- (let* ((expanded (org-babel-expand-body:clojure body params cljs-p))
+ (let* ((expanded (org-babel-expand-body:clojure body params))
+ (prepared (ob-clojure--prepare-for-evaluation expanded params cljs-p))
(result-params (cdr (assq :result-params params)))
result)
(setq result
(cond
((eq org-babel-clojure-backend 'inf-clojure)
- (ob-clojure-eval-with-inf-clojure expanded params))
+ (ob-clojure-eval-with-inf-clojure prepared params))
((eq org-babel-clojure-backend 'clojure-cli)
- (ob-clojure-eval-with-cmd ob-clojure-cli-command expanded))
+ (ob-clojure-eval-with-cmd ob-clojure-cli-command prepared))
((eq org-babel-clojure-backend 'babashka)
- (ob-clojure-eval-with-cmd ob-clojure-babashka-command expanded))
+ (ob-clojure-eval-with-cmd ob-clojure-babashka-command prepared))
((eq org-babel-clojure-backend 'nbb)
- (ob-clojure-eval-with-cmd ob-clojure-nbb-command expanded))
+ (ob-clojure-eval-with-cmd ob-clojure-nbb-command prepared))
((eq org-babel-clojure-backend 'cider)
- (ob-clojure-eval-with-cider expanded params cljs-p))
+ (ob-clojure-eval-with-cider prepared params cljs-p))
((eq org-babel-clojure-backend 'slime)
- (ob-clojure-eval-with-slime expanded params))
+ (ob-clojure-eval-with-slime prepared params))
(t (user-error "Invalid backend"))))
(org-babel-result-cond result-params
result
diff --git c/testing/lisp/test-ob-clojure.el w/testing/lisp/test-ob-clojure.el
index 4784f33c6..e4e02f783 100644
--- c/testing/lisp/test-ob-clojure.el
+++ w/testing/lisp/test-ob-clojure.el
@@ -23,13 +23,37 @@
;; Org tests for ob-clojure.el live here
;;; Code:
-
(require 'org-test "../testing/org-test")
(unless (featurep 'ob-clojure)
(signal 'missing-test-dependency '("Support for Clojure code blocks")))
-;; FIXME: The old tests where totally off. We need to write new tests.
+;; tangle
+(ert-deftest ob-clojure/org-babel-tangle ()
+ "org-babel-tangle returns the exact body of the source block."
+ (org-test-with-temp-text-in-file
+ "#+begin_src clojure :tangle \"tangle.clj\" :results value\n
+(+ 1 2)\n#+end_src"
+ (unwind-protect
+ (progn (org-babel-tangle)
+ (with-temp-buffer
+ (insert-file-contents "tangle.clj")
+ (let ((tangled (buffer-string)))
+ (should
+ (string-match-p "^(\\+ 1 2)\\\n$" tangled)))))
+ (delete-file "tangle.clj"))))
+
+;; execute
+(ert-deftest ob-clojure/org-babel-execute ()
+ "org-babel-execute:clojure correctly handles :result-params."
+ (should (equal 3
+ (org-babel-execute:clojure
+ "(+ 1 2)"
+ '((:result-params "value")))))
+ (should (equal ""
+ (org-babel-execute:clojure
+ "(+ 1 2)"
+ '((:result-params "output"))))))
(provide 'test-ob-clojure)
