* [patch] ob-clojure: Fix results output @ 2023-03-09 15:40 Daniel Kraus 2023-03-10 12:35 ` Ihor Radchenko 0 siblings, 1 reply; 15+ messages in thread From: Daniel Kraus @ 2023-03-09 15:40 UTC (permalink / raw) To: emacs-orgmode [-- Attachment #1: Type: text/plain, Size: 418 bytes --] Hi, attached is a bigger patch that fixes the result output of ob-clojure. The commit message contains examples what's currently wrong and what's fixed. This is a breaking change though. So if someone before relied on the fact that, e.g. cider, returns the result of every expression in every line instead of only the last one, they get a different result now. Is it ok to install? Other feedback? Cheers, Daniel [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-ob-clojure.el-Fix-results-output-support-clojure-cli.patch --] [-- Type: text/x-patch, Size: 11377 bytes --] From 77783d864d81ef1d962c302523d7c588f248c088 Mon Sep 17 00:00:00 2001 From: Daniel Kraus <daniel@kraus.my> Date: Thu, 9 Mar 2023 16:11:27 +0100 Subject: [PATCH] ob-clojure.el: Fix results output, support clojure-cli * lisp/ob-clojure.el (org-babel-clojure-backend): Add support for clojure-cli. * lisp/ob-clojure.el (org-babel-clojurescript-backend): Move nbb to clojurescript. * lisp/ob-clojure.el (org-babel-expand-body:clojure) * lisp/ob-clojure.el (ob-clojure-eval-with-cider): Return only the last expression when :results is not set or value, and return only stdout when :results is set to output. * lisp/ob-clojure.el (ob-clojure-eval-with-cmd): Rename function as it is not only for babashka. * lisp/ob-clojure.el (org-babel-execute:clojure): Differentiate between Clojure and ClojureScript source blocks. The problem was that the ob-clojure results where not correctly taking the results parameter into account. E.g. with the cider backend, you would get all printed or returned values for each line in your block: (def small-map {:a 2 :b 4 :c 8}) {:some :map} (prn :xx) (:b small-map) | #'user/small-map | | {:some :map} | | 4 | or for babashka you would only get the printed values but not the last return value: (def small-map {:a 2 :b 4 :c 8}) {:some :map} (prn :xx) (:b small-map) : :xx Now when you specify :results value, the result is only the last returned value, and with :results output you get all values printed to stdout. So the examples above would all result in the same: (def small-map {:a 2 :b 4 :c 8}) {:some :map} (prn :xx) (:b small-map) : 4 --- lisp/ob-clojure.el | 120 +++++++++++++++++++++++++++++---------------- 1 file changed, 77 insertions(+), 43 deletions(-) diff --git a/lisp/ob-clojure.el b/lisp/ob-clojure.el index 5f589db00..6345927d6 100644 --- a/lisp/ob-clojure.el +++ b/lisp/ob-clojure.el @@ -25,20 +25,21 @@ ;;; Commentary: -;; Support for evaluating Clojure code +;; Support for evaluating Clojure / ClojureScript code. ;; Requirements: ;; - Clojure (at least 1.2.0) ;; - clojure-mode -;; - inf-clojure, Cider, SLIME, babashka or nbb +;; - babashka, nbb, Clojure CLI tools, Cider, inf-clojure or SLIME ;; For clojure-mode, see https://github.com/clojure-emacs/clojure-mode -;; For inf-clojure, see https://github.com/clojure-emacs/inf-clojure -;; For Cider, see https://github.com/clojure-emacs/cider -;; For SLIME, see https://slime.common-lisp.dev ;; For babashka, see https://github.com/babashka/babashka ;; For nbb, see https://github.com/babashka/nbb +;; For Clojure CLI tools, see https://clojure.org/guides/deps_and_cli +;; For Cider, see https://github.com/clojure-emacs/cider +;; For inf-clojure, see https://github.com/clojure-emacs/inf-clojure +;; For SLIME, see https://slime.common-lisp.dev ;; For SLIME, the best way to install its components is by following ;; the directions as set out by Phil Hagelberg (Technomancy) on the @@ -78,7 +79,7 @@ defvar org-babel-header-args:clojurescript (defcustom org-babel-clojure-backend (cond ((executable-find "bb") 'babashka) - ((executable-find "nbb") 'nbb) + ((executable-find "clojure") 'clojure-cli) ((featurep 'cider) 'cider) ((featurep 'inf-clojure) 'inf-clojure) ((featurep 'slime) 'slime) @@ -87,11 +88,24 @@ defcustom org-babel-clojure-backend :group 'org-babel :package-version '(Org . "9.6") :type '(choice - (const :tag "inf-clojure" inf-clojure) + (const :tag "babashka" babashka) + (const :tag "clojure-cli" clojure-cli) (const :tag "cider" cider) + (const :tag "inf-clojure" inf-clojure) (const :tag "slime" slime) - (const :tag "babashka" babashka) + (const :tag "Not configured yet" nil))) + +(defcustom org-babel-clojurescript-backend + (cond + ((or (executable-find "nbb") (executable-find "npx")) 'nbb) + ((featurep 'cider) 'cider) + (t nil)) + "Backend used to evaluate Clojure code blocks." + :group 'org-babel + :package-version '(Org . "9.6") + :type '(choice (const :tag "nbb" nbb) + (const :tag "cider" cider) (const :tag "Not configured yet" nil))) (defcustom org-babel-clojure-default-ns "user" @@ -105,15 +119,24 @@ defcustom ob-clojure-babashka-command :group 'org-babel :package-version '(Org . "9.6")) -(defcustom ob-clojure-nbb-command (executable-find "nbb") +(defcustom ob-clojure-nbb-command (or (executable-find "nbb") + (when-let (npx (executable-find "npx")) + (concat npx " nbb"))) "Path to the nbb executable." :type '(choice file (const nil)) + :group 'org-babel) + +(defcustom ob-clojure-cli-command (when-let (cmd (executable-find "clojure")) + (concat cmd " -M")) + "Clojure CLI command used to execute source blocks." + :type 'file :group 'org-babel :package-version '(Org . "9.6")) (defun org-babel-expand-body:clojure (body params) "Expand BODY according to PARAMS, return the expanded body." (let* ((vars (org-babel--get-vars params)) + (cljs-p (string= (cdr (assq :target params)) "cljs")) (backend-override (cdr (assq :backend params))) (org-babel-clojure-backend (cond @@ -146,10 +169,26 @@ defun org-babel-expand-body:clojure vars "\n ") body)))))) - (if (or (member "code" result-params) - (member "pp" result-params)) - (format "(clojure.pprint/pprint (do %s))" body) - 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 "))")))) (defvar ob-clojure-inf-clojure-filter-out) (defvar ob-clojure-inf-clojure-tmp-output) @@ -228,29 +267,15 @@ defun ob-clojure-eval-with-inf-clojure (defun ob-clojure-eval-with-cider (expanded params) "Evaluate EXPANDED code block with PARAMS using cider." (org-require-package 'cider "Cider") - (let ((connection (cider-current-connection (cdr (assq :target params)))) - (result-params (cdr (assq :result-params params))) - result0) + (let ((connection (cider-current-connection (cdr (assq :target params))))) (unless connection (sesman-start-session 'CIDER)) (if (not connection) ;; Display in the result instead of using `user-error' - (setq result0 "Please reevaluate when nREPL is connected") - (ob-clojure-with-temp-expanded expanded params - (let ((response (nrepl-sync-request:eval exp connection))) - (push (or (nrepl-dict-get response "root-ex") - (nrepl-dict-get response "ex") - (nrepl-dict-get - response (if (or (member "output" result-params) - (member "pp" result-params)) - "out" - "value"))) - result0))) - (ob-clojure-string-or-list - ;; Filter out s-expressions that return nil (string "nil" - ;; from nrepl eval) or comment forms (actual nil from nrepl) - (reverse (delete "" (mapcar (lambda (r) - (replace-regexp-in-string "nil" "" (or r ""))) - result0))))))) + "Please reevaluate when nREPL is connected" + (let ((response (nrepl-sync-request:eval expanded connection))) + (or (nrepl-dict-get response "root-ex") + (nrepl-dict-get response "ex") + (nrepl-dict-get response "out")))))) (defun ob-clojure-eval-with-slime (expanded params) "Evaluate EXPANDED code block with PARAMS using slime." @@ -262,24 +287,30 @@ defun ob-clojure-eval-with-slime ,(buffer-substring-no-properties (point-min) (point-max))) (cdr (assq :package params))))) -(defun ob-clojure-eval-with-babashka (bb expanded) - "Evaluate EXPANDED code block using BB (babashka or nbb)." - (let ((script-file (org-babel-temp-file "clojure-bb-script-" ".clj"))) +(defun ob-clojure-eval-with-cmd (cmd expanded) + "Evaluate EXPANDED code block using CMD (babashka, clojure or nbb)." + (let ((script-file (org-babel-temp-file "clojure-cmd-script-" ".clj"))) (with-temp-file script-file (insert expanded)) (org-babel-eval - (format "%s %s" bb (org-babel-process-file-name script-file)) + (format "%s %s" cmd (org-babel-process-file-name script-file)) ""))) (defun org-babel-execute:clojure (body params) "Execute the BODY block of Clojure code with PARAMS using Babel." (let* ((backend-override (cdr (assq :backend params))) + (cljs-p (string= (cdr (assq :target params)) "cljs")) (org-babel-clojure-backend (cond (backend-override (intern backend-override)) - (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"))))) + (org-babel-clojure-backend (if cljs-p + org-babel-clojurescript-backend + org-babel-clojure-backend)) + (t (user-error "You need to customize `%S' +or set the `:backend' header argument" + (if cljs-p + org-babel-clojurescript-backend + org-babel-clojure-backend)))))) (let* ((expanded (org-babel-expand-body:clojure body params)) (result-params (cdr (assq :result-params params))) result) @@ -287,14 +318,17 @@ defun org-babel-execute:clojure (cond ((eq org-babel-clojure-backend 'inf-clojure) (ob-clojure-eval-with-inf-clojure expanded params)) + ((eq org-babel-clojure-backend 'clojure-cli) + (ob-clojure-eval-with-cmd ob-clojure-cli-command expanded)) ((eq org-babel-clojure-backend 'babashka) - (ob-clojure-eval-with-babashka ob-clojure-babashka-command expanded)) + (ob-clojure-eval-with-cmd ob-clojure-babashka-command expanded)) ((eq org-babel-clojure-backend 'nbb) - (ob-clojure-eval-with-babashka ob-clojure-nbb-command expanded)) + (ob-clojure-eval-with-cmd ob-clojure-nbb-command expanded)) ((eq org-babel-clojure-backend 'cider) (ob-clojure-eval-with-cider expanded params)) ((eq org-babel-clojure-backend 'slime) - (ob-clojure-eval-with-slime expanded params)))) + (ob-clojure-eval-with-slime expanded params)) + (t (user-error "Invalid backend")))) (org-babel-result-cond result-params result (condition-case nil (org-babel-script-escape result) -- 2.39.2 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [patch] ob-clojure: Fix results output 2023-03-09 15:40 [patch] ob-clojure: Fix results output Daniel Kraus @ 2023-03-10 12:35 ` Ihor Radchenko 2023-03-13 14:01 ` Daniel Kraus 0 siblings, 1 reply; 15+ messages in thread From: Ihor Radchenko @ 2023-03-10 12:35 UTC (permalink / raw) To: Daniel Kraus; +Cc: emacs-orgmode Daniel Kraus <daniel@kraus.my> writes: > attached is a bigger patch that fixes the result output of ob-clojure. > The commit message contains examples what's currently wrong > and what's fixed. > This is a breaking change though. > So if someone before relied on the fact that, e.g. cider, returns > the result of every expression in every line instead of only the > last one, they get a different result now. The current behaviour is a bug anyway: ‘value’ ... When evaluating the code block in a session (see *note Environment of a Code Block::), Org passes the code to an interpreter running as an interactive Emacs inferior process. Org gets the value from the source code interpreter’s last statement output. Org has to use language-specific methods to obtain the value. For example, from the variable ‘_’ in Ruby, and the value of ‘.Last.value’ in R. > Is it ok to install? > Other feedback? You need to document the changes in ORG-NEWS. Also, see inline comments below. > Subject: [PATCH] ob-clojure.el: Fix results output, support clojure-cli Does it have to be a single commit? Not a big deal, but two separate commits would be cleaner. > -;; Support for evaluating Clojure code > +;; Support for evaluating Clojure / ClojureScript code. This is an important new feature that should be announced clearly in ORG-NEWS. Also, make sure that https://orgmode.org/worg/org-contrib/babel/languages/ob-doc-clojure.html is up-to-date. > (defcustom org-babel-clojure-backend (cond > ((executable-find "bb") 'babashka) > - ((executable-find "nbb") 'nbb) > + ((executable-find "clojure") 'clojure-cli) > ((featurep 'cider) 'cider) > ((featurep 'inf-clojure) 'inf-clojure) > ((featurep 'slime) 'slime) > @@ -87,11 +88,24 @@ defcustom org-babel-clojure-backend > :group 'org-babel > :package-version '(Org . "9.6") Org 9.7. This is a new feature to come in the next release. > +(defcustom org-babel-clojurescript-backend > + (cond > + ((or (executable-find "nbb") (executable-find "npx")) 'nbb) > + ((featurep 'cider) 'cider) > + (t nil)) > + "Backend used to evaluate Clojure code blocks." > + :group 'org-babel > + :package-version '(Org . "9.6") 9.7 New customization to be announced in ORG-NEWS. > -(defcustom ob-clojure-nbb-command (executable-find "nbb") > +(defcustom ob-clojure-nbb-command (or (executable-find "nbb") > + (when-let (npx (executable-find "npx")) > + (concat npx " nbb"))) > "Path to the nbb executable." > :type '(choice file (const nil)) > + :group 'org-babel) It is no longer a path, despite what is claimed in the docstring. :type definition is also not accurate. Also, update :package-version. > +(defcustom ob-clojure-cli-command (when-let (cmd (executable-find "clojure")) > + (concat cmd " -M")) > + "Clojure CLI command used to execute source blocks." > + :type 'file > :group 'org-babel > :package-version '(Org . "9.6")) 9.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)) > + (cljs-p (string= (cdr (assq :target params)) "cljs")) Note: I do not see :target header arg being documented in https://orgmode.org/worg/org-contrib/babel/languages/ob-doc-clojure.html > -(defun ob-clojure-eval-with-babashka (bb expanded) > - "Evaluate EXPANDED code block using BB (babashka or nbb)." > - (let ((script-file (org-babel-temp-file "clojure-bb-script-" ".clj"))) This will remove a non-private function. May you leave a fallback obsolete alias to not break third-party code that calls the old function name? -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [patch] ob-clojure: Fix results output 2023-03-10 12:35 ` Ihor Radchenko @ 2023-03-13 14:01 ` Daniel Kraus 2023-03-14 12:35 ` Ihor Radchenko 0 siblings, 1 reply; 15+ messages in thread From: Daniel Kraus @ 2023-03-13 14:01 UTC (permalink / raw) To: Ihor Radchenko; +Cc: emacs-orgmode [-- Attachment #1: Type: text/plain, Size: 4239 bytes --] Hi! Ihor Radchenko <yantar92@posteo.net> writes: > You need to document the changes in ORG-NEWS. I added an entry about the new ClojureScript change. I guess the other (main) part of the patch is a bugfix and will not be documented in ORG-NEWS. > Also, see inline comments below. > >> Subject: [PATCH] ob-clojure.el: Fix results output, support clojure-cli > > Does it have to be a single commit? Not a big deal, but two separate > commits would be cleaner. I saw that as well but unfortunately I did it all in the same go and then touch the same code parts, so separating them in individual working commits, while not a huge task, is still some work with not much benefit. >> -;; Support for evaluating Clojure code >> +;; Support for evaluating Clojure / ClojureScript code. > > This is an important new feature that should be announced clearly in ORG-NEWS. ClojureScript source blocks where supported before, but just no way to set the backend for it separately. See the ORG-NEWS in the new attached patch. > Also, make sure that > https://orgmode.org/worg/org-contrib/babel/languages/ob-doc-clojure.html > is up-to-date. Good reminder. I send a PR for this when this patch is installed?! >> (defcustom org-babel-clojure-backend (cond >> ((executable-find "bb") 'babashka) >> - ((executable-find "nbb") 'nbb) >> + ((executable-find "clojure") 'clojure-cli) >> ((featurep 'cider) 'cider) >> ((featurep 'inf-clojure) 'inf-clojure) >> ((featurep 'slime) 'slime) >> @@ -87,11 +88,24 @@ defcustom org-babel-clojure-backend >> :group 'org-babel >> :package-version '(Org . "9.6") > > Org 9.7. This is a new feature to come in the next release. Fixed. >> +(defcustom org-babel-clojurescript-backend >> + (cond >> + ((or (executable-find "nbb") (executable-find "npx")) 'nbb) >> + ((featurep 'cider) 'cider) >> + (t nil)) >> + "Backend used to evaluate Clojure code blocks." >> + :group 'org-babel >> + :package-version '(Org . "9.6") > > 9.7 > New customization to be announced in ORG-NEWS. Fixed. >> -(defcustom ob-clojure-nbb-command (executable-find "nbb") >> +(defcustom ob-clojure-nbb-command (or (executable-find "nbb") >> + (when-let (npx (executable-find "npx")) >> + (concat npx " nbb"))) >> "Path to the nbb executable." >> :type '(choice file (const nil)) >> + :group 'org-babel) > > It is no longer a path, despite what is claimed in the docstring. > :type definition is also not accurate. > Also, update :package-version. Fixed. >> +(defcustom ob-clojure-cli-command (when-let (cmd (executable-find "clojure")) >> + (concat cmd " -M")) >> + "Clojure CLI command used to execute source blocks." >> + :type 'file >> :group 'org-babel >> :package-version '(Org . "9.6")) > > 9.7 Fixed. >> (defun org-babel-expand-body:clojure (body params) >> "Expand BODY according to PARAMS, return the expanded body." >> (let* ((vars (org-babel--get-vars params)) >> + (cljs-p (string= (cdr (assq :target params)) "cljs")) > > Note: I do not see :target header arg being documented in > https://orgmode.org/worg/org-contrib/babel/languages/ob-doc-clojure.html This was apparently a kludge that ob-clojure used to evaluate ClojureScript in the normal clojure:execute function. I simply used the same kludge where I need to check for cljs, but after reviewing it's not really necessary and I removed the :target parameter completely. As this was undocumented I guess it's ok to remove?! >> -(defun ob-clojure-eval-with-babashka (bb expanded) >> - "Evaluate EXPANDED code block using BB (babashka or nbb)." >> - (let ((script-file (org-babel-temp-file "clojure-bb-script-" ".clj"))) > > This will remove a non-private function. May you leave a fallback > obsolete alias to not break third-party code that calls the old function > name? I created an obsolete-function-alias. Attached is the new patch with the changes. Thanks, Daniel [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-ob-clojure.el-Fix-results-output-support-clojure-cli.patch --] [-- Type: text/x-patch, Size: 13366 bytes --] From 3ad98fa88f6ebd4ae1b2b41d66cca9e9d6e1875e Mon Sep 17 00:00:00 2001 From: Daniel Kraus <daniel@kraus.my> Date: Thu, 9 Mar 2023 16:11:27 +0100 Subject: [PATCH] ob-clojure.el: Fix results output, support clojure-cli * lisp/ob-clojure.el (org-babel-clojure-backend): Add support for clojure-cli. * lisp/ob-clojure.el (org-babel-clojurescript-backend): Move nbb to clojurescript. * lisp/ob-clojure.el (org-babel-expand-body:clojure) * lisp/ob-clojure.el (ob-clojure-eval-with-cider): Return only the last expression when :results is not set or value, and return only stdout when :results is set to output. * lisp/ob-clojure.el (ob-clojure-eval-with-cmd): Rename function as it is not only for babashka. * lisp/ob-clojure.el (org-babel-execute:clojure): Differentiate between Clojure and ClojureScript source blocks. The problem was that the ob-clojure results where not correctly taking the results parameter into account. E.g. with the cider backend, you would get all printed or returned values for each line in your block: (def small-map {:a 2 :b 4 :c 8}) {:some :map} (prn :xx) (:b small-map) | #'user/small-map | | {:some :map} | | 4 | or for babashka you would only get the printed values but not the last return value: (def small-map {:a 2 :b 4 :c 8}) {:some :map} (prn :xx) (:b small-map) : :xx Now when you specify :results value, the result is only the last returned value, and with :results output you get all values printed to stdout. So the examples above would all result in the same: (def small-map {:a 2 :b 4 :c 8}) {:some :map} (prn :xx) (:b small-map) : 4 --- etc/ORG-NEWS | 9 +++ lisp/ob-clojure.el | 150 ++++++++++++++++++++++++++++----------------- 2 files changed, 104 insertions(+), 55 deletions(-) diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS index b9d7b3459..3319de0a1 100644 --- a/etc/ORG-NEWS +++ b/etc/ORG-NEWS @@ -96,6 +96,15 @@ The face ~org-agenda-calendar-daterange~ is used to show entries with a date range in the agenda. It inherits from the default face in order to remain backward-compatible. +*** New ~org-babel-clojurescript-backend~ option to choose ClojureScript backend + +Before, a ClojureScript source block used the same backend as Clojure, +configured in ~org-babel-clojure-backend~ and relied on an undocumented +~:target~ paramter. + +Now, there's ~org-babel-clojurescript-backend~ to determine the +backend used for evaluation of ClojureScript. + ** New features *** ~org-metaup~ and ~org-metadown~ now act on headings in region diff --git a/lisp/ob-clojure.el b/lisp/ob-clojure.el index 5f589db00..8ae1395d2 100644 --- a/lisp/ob-clojure.el +++ b/lisp/ob-clojure.el @@ -25,20 +25,21 @@ ;;; Commentary: -;; Support for evaluating Clojure code +;; Support for evaluating Clojure / ClojureScript code. ;; Requirements: ;; - Clojure (at least 1.2.0) ;; - clojure-mode -;; - inf-clojure, Cider, SLIME, babashka or nbb +;; - babashka, nbb, Clojure CLI tools, Cider, inf-clojure or SLIME ;; For clojure-mode, see https://github.com/clojure-emacs/clojure-mode -;; For inf-clojure, see https://github.com/clojure-emacs/inf-clojure -;; For Cider, see https://github.com/clojure-emacs/cider -;; For SLIME, see https://slime.common-lisp.dev ;; For babashka, see https://github.com/babashka/babashka ;; For nbb, see https://github.com/babashka/nbb +;; For Clojure CLI tools, see https://clojure.org/guides/deps_and_cli +;; For Cider, see https://github.com/clojure-emacs/cider +;; For inf-clojure, see https://github.com/clojure-emacs/inf-clojure +;; For SLIME, see https://slime.common-lisp.dev ;; For SLIME, the best way to install its components is by following ;; the directions as set out by Phil Hagelberg (Technomancy) on the @@ -78,20 +79,33 @@ defvar org-babel-header-args:clojurescript (defcustom org-babel-clojure-backend (cond ((executable-find "bb") 'babashka) - ((executable-find "nbb") 'nbb) + ((executable-find "clojure") 'clojure-cli) ((featurep 'cider) 'cider) ((featurep 'inf-clojure) 'inf-clojure) ((featurep 'slime) 'slime) (t nil)) "Backend used to evaluate Clojure code blocks." :group 'org-babel - :package-version '(Org . "9.6") + :package-version '(Org . "9.7") :type '(choice - (const :tag "inf-clojure" inf-clojure) + (const :tag "babashka" babashka) + (const :tag "clojure-cli" clojure-cli) (const :tag "cider" cider) + (const :tag "inf-clojure" inf-clojure) (const :tag "slime" slime) - (const :tag "babashka" babashka) + (const :tag "Not configured yet" nil))) + +(defcustom org-babel-clojurescript-backend + (cond + ((or (executable-find "nbb") (executable-find "npx")) 'nbb) + ((featurep 'cider) 'cider) + (t nil)) + "Backend used to evaluate Clojure code blocks." + :group 'org-babel + :package-version '(Org . "9.7") + :type '(choice (const :tag "nbb" nbb) + (const :tag "cider" cider) (const :tag "Not configured yet" nil))) (defcustom org-babel-clojure-default-ns "user" @@ -105,14 +119,24 @@ defcustom ob-clojure-babashka-command :group 'org-babel :package-version '(Org . "9.6")) -(defcustom ob-clojure-nbb-command (executable-find "nbb") +(defcustom ob-clojure-nbb-command (or (executable-find "nbb") + (when-let (npx (executable-find "npx")) + (concat npx " nbb"))) "Path to the nbb executable." - :type '(choice file (const nil)) + :type '(choice string (const nil)) :group 'org-babel - :package-version '(Org . "9.6")) + :package-version '(Org . "9.7")) -(defun org-babel-expand-body:clojure (body params) - "Expand BODY according to PARAMS, return the expanded body." +(defcustom ob-clojure-cli-command (when-let (cmd (executable-find "clojure")) + (concat cmd " -M")) + "Clojure CLI command used to execute source blocks." + :type 'string + :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." (let* ((vars (org-babel--get-vars params)) (backend-override (cdr (assq :backend params))) (org-babel-clojure-backend @@ -146,10 +170,26 @@ defun org-babel-expand-body:clojure vars "\n ") body)))))) - (if (or (member "code" result-params) - (member "pp" result-params)) - (format "(clojure.pprint/pprint (do %s))" body) - 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 "))")))) (defvar ob-clojure-inf-clojure-filter-out) (defvar ob-clojure-inf-clojure-tmp-output) @@ -225,32 +265,19 @@ defun ob-clojure-eval-with-inf-clojure s)) (reverse ob-clojure-inf-clojure-tmp-output))))) -(defun ob-clojure-eval-with-cider (expanded params) - "Evaluate EXPANDED code block with PARAMS using cider." +(defun ob-clojure-eval-with-cider (expanded params &optional cljs-p) + "Evaluate EXPANDED code block with PARAMS using cider. +When CLJS-P is non-nil, use a cljs connection instead of clj." (org-require-package 'cider "Cider") - (let ((connection (cider-current-connection (cdr (assq :target params)))) - (result-params (cdr (assq :result-params params))) - result0) + (let ((connection (cider-current-connection (if cljs-p "cljs" "clj")))) (unless connection (sesman-start-session 'CIDER)) (if (not connection) ;; Display in the result instead of using `user-error' - (setq result0 "Please reevaluate when nREPL is connected") - (ob-clojure-with-temp-expanded expanded params - (let ((response (nrepl-sync-request:eval exp connection))) - (push (or (nrepl-dict-get response "root-ex") - (nrepl-dict-get response "ex") - (nrepl-dict-get - response (if (or (member "output" result-params) - (member "pp" result-params)) - "out" - "value"))) - result0))) - (ob-clojure-string-or-list - ;; Filter out s-expressions that return nil (string "nil" - ;; from nrepl eval) or comment forms (actual nil from nrepl) - (reverse (delete "" (mapcar (lambda (r) - (replace-regexp-in-string "nil" "" (or r ""))) - result0))))))) + "Please reevaluate when nREPL is connected" + (let ((response (nrepl-sync-request:eval expanded connection))) + (or (nrepl-dict-get response "root-ex") + (nrepl-dict-get response "ex") + (nrepl-dict-get response "out")))))) (defun ob-clojure-eval-with-slime (expanded params) "Evaluate EXPANDED code block with PARAMS using slime." @@ -262,39 +289,52 @@ defun ob-clojure-eval-with-slime ,(buffer-substring-no-properties (point-min) (point-max))) (cdr (assq :package params))))) -(defun ob-clojure-eval-with-babashka (bb expanded) - "Evaluate EXPANDED code block using BB (babashka or nbb)." - (let ((script-file (org-babel-temp-file "clojure-bb-script-" ".clj"))) +(defun ob-clojure-eval-with-cmd (cmd expanded) + "Evaluate EXPANDED code block using CMD (babashka, clojure or nbb)." + (let ((script-file (org-babel-temp-file "clojure-cmd-script-" ".clj"))) (with-temp-file script-file (insert expanded)) (org-babel-eval - (format "%s %s" bb (org-babel-process-file-name script-file)) + (format "%s %s" cmd (org-babel-process-file-name script-file)) ""))) -(defun org-babel-execute:clojure (body params) - "Execute the BODY block of Clojure code with PARAMS using Babel." +(define-obsolete-function-alias 'ob-clojure-eval-with-babashka + #'ob-clojure-eval-with-cmd "9.7") + +(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 +instead of Clojure." (let* ((backend-override (cdr (assq :backend params))) (org-babel-clojure-backend (cond (backend-override (intern backend-override)) - (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"))))) - (let* ((expanded (org-babel-expand-body:clojure body params)) + (org-babel-clojure-backend (if cljs-p + org-babel-clojurescript-backend + org-babel-clojure-backend)) + (t (user-error "You need to customize `%S' +or set the `:backend' header argument" + (if cljs-p + org-babel-clojurescript-backend + org-babel-clojure-backend)))))) + (let* ((expanded (org-babel-expand-body:clojure body 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)) + ((eq org-babel-clojure-backend 'clojure-cli) + (ob-clojure-eval-with-cmd ob-clojure-cli-command expanded)) ((eq org-babel-clojure-backend 'babashka) - (ob-clojure-eval-with-babashka ob-clojure-babashka-command expanded)) + (ob-clojure-eval-with-cmd ob-clojure-babashka-command expanded)) ((eq org-babel-clojure-backend 'nbb) - (ob-clojure-eval-with-babashka ob-clojure-nbb-command expanded)) + (ob-clojure-eval-with-cmd ob-clojure-nbb-command expanded)) ((eq org-babel-clojure-backend 'cider) - (ob-clojure-eval-with-cider expanded params)) + (ob-clojure-eval-with-cider expanded params cljs-p)) ((eq org-babel-clojure-backend 'slime) - (ob-clojure-eval-with-slime expanded params)))) + (ob-clojure-eval-with-slime expanded params)) + (t (user-error "Invalid backend")))) (org-babel-result-cond result-params result (condition-case nil (org-babel-script-escape result) @@ -302,7 +342,7 @@ defun org-babel-execute:clojure (defun org-babel-execute:clojurescript (body params) "Evaluate BODY with PARAMS as ClojureScript code." - (org-babel-execute:clojure body (cons '(:target . "cljs") params))) + (org-babel-execute:clojure body params t)) (provide 'ob-clojure) -- 2.39.2 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [patch] ob-clojure: Fix results output 2023-03-13 14:01 ` Daniel Kraus @ 2023-03-14 12:35 ` Ihor Radchenko 2023-03-14 13:38 ` Daniel Kraus 0 siblings, 1 reply; 15+ messages in thread From: Ihor Radchenko @ 2023-03-14 12:35 UTC (permalink / raw) To: Daniel Kraus; +Cc: emacs-orgmode Daniel Kraus <daniel@kraus.my> writes: >> Also, make sure that >> https://orgmode.org/worg/org-contrib/babel/languages/ob-doc-clojure.html >> is up-to-date. > > Good reminder. I send a PR for this when this patch is installed?! Or together. Either way is fine. >> Note: I do not see :target header arg being documented in >> https://orgmode.org/worg/org-contrib/babel/languages/ob-doc-clojure.html > > This was apparently a kludge that ob-clojure used to evaluate ClojureScript > in the normal clojure:execute function. > I simply used the same kludge where I need to check for cljs, but after > reviewing it's not really necessary and I removed the :target parameter > completely. As this was undocumented I guess it's ok to remove?! Yes, it is OK to remove what is undocumented. We may still announce the change though. >>> -(defun ob-clojure-eval-with-babashka (bb expanded) >>> - "Evaluate EXPANDED code block using BB (babashka or nbb)." >>> - (let ((script-file (org-babel-temp-file "clojure-bb-script-" ".clj"))) >> >> This will remove a non-private function. May you leave a fallback >> obsolete alias to not break third-party code that calls the old function >> name? > > I created an obsolete-function-alias. It should better go to org-compat.el. > Attached is the new patch with the changes. Thanks! A few more comments below. > +*** New ~org-babel-clojurescript-backend~ option to choose ClojureScript backend > + > +Before, a ClojureScript source block used the same backend as Clojure, > +configured in ~org-babel-clojure-backend~ and relied on an undocumented > +~:target~ paramter. > + > +Now, there's ~org-babel-clojurescript-backend~ to determine the > +backend used for evaluation of ClojureScript. What about the new customization `ob-clojure-cli-command'? > -(defcustom ob-clojure-nbb-command (executable-find "nbb") > +(defcustom ob-clojure-nbb-command (or (executable-find "nbb") > + (when-let (npx (executable-find "npx")) > + (concat npx " nbb"))) > "Path to the nbb executable." This is not a path anymore, when the value is "npx nbb". Can just use "Command to invoke nbb executable". -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [patch] ob-clojure: Fix results output 2023-03-14 12:35 ` Ihor Radchenko @ 2023-03-14 13:38 ` Daniel Kraus 2023-03-14 14:27 ` Daniel Kraus 0 siblings, 1 reply; 15+ messages in thread From: Daniel Kraus @ 2023-03-14 13:38 UTC (permalink / raw) To: Ihor Radchenko; +Cc: emacs-orgmode [-- Attachment #1: Type: text/plain, Size: 1384 bytes --] Hi! Ihor Radchenko <yantar92@posteo.net> writes: > Daniel Kraus <daniel@kraus.my> writes: > >> This was apparently a kludge that ob-clojure used to evaluate ClojureScript >> in the normal clojure:execute function. >> I simply used the same kludge where I need to check for cljs, but after >> reviewing it's not really necessary and I removed the :target parameter >> completely. As this was undocumented I guess it's ok to remove?! > > Yes, it is OK to remove what is undocumented. We may still announce the > change though. I added an entry to ORG-NEWS under Misc. >>>> -(defun ob-clojure-eval-with-babashka (bb expanded) >> I created an obsolete-function-alias. > > It should better go to org-compat.el. Moved the alias to org-compat. I wasn't sure where to put it exactly. It's now in the ~Obsolete aliases~ page. > What about the new customization `ob-clojure-cli-command'? I added a news entry in ORG-NEWS. >> +(defcustom ob-clojure-nbb-command (or (executable-find "nbb") >> + (when-let (npx (executable-find "npx")) >> + (concat npx " nbb"))) >> "Path to the nbb executable." > > This is not a path anymore, when the value is "npx nbb". > Can just use "Command to invoke nbb executable". Fixed. Attached is the new patch with the changes. Thanks for your review and guidance, Daniel [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-lisp-ob-sql.el-Add-support-for-Athena.patch --] [-- Type: text/x-patch, Size: 2150 bytes --] From ddace051205d20b24c047962ca9d1335bdd90284 Mon Sep 17 00:00:00 2001 From: Daniel Kraus <daniel@kraus.my> Date: Mon, 16 Jan 2023 11:35:02 +0100 Subject: [PATCH] lisp/ob-sql.el: Add support for Athena * lisp/ob-sql.el (org-babel-execute:sql): Add support for Athena --- lisp/ob-sql.el | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/lisp/ob-sql.el b/lisp/ob-sql.el index 39a4573a5..640ecb2c0 100644 --- a/lisp/ob-sql.el +++ b/lisp/ob-sql.el @@ -53,14 +53,15 @@ ;; - rowname-names ;; ;; Engines supported: -;; - mysql +;; - athena ;; - dbi ;; - mssql -;; - sqsh -;; - postgresql (postgres) +;; - mysql ;; - oracle -;; - vertica +;; - postgresql (postgres) ;; - saphana +;; - sqsh +;; - vertica ;; ;; TODO: ;; @@ -254,6 +255,11 @@ This function is called by `org-babel-execute-src-block'." (org-babel-temp-file "sql-out-"))) (header-delim "") (command (cl-case (intern engine) + (athena (format "athenacli %s -e %s %s > %s" + (or cmdline "") + (org-babel-process-file-name in-file) + database + (org-babel-process-file-name out-file))) (dbi (format "dbish --batch %s < %s | sed '%s' > %s" (or cmdline "") (org-babel-process-file-name in-file) @@ -352,7 +358,7 @@ SET COLSEP '|' (progn (insert-file-contents-literally out-file) (buffer-string))) (with-temp-buffer (cond - ((memq (intern engine) '(dbi mysql postgresql postgres saphana sqsh vertica)) + ((memq (intern engine) '(athena dbi mysql postgresql postgres saphana sqsh vertica)) ;; Add header row delimiter after column-names header in first line (cond (colnames-p @@ -377,7 +383,7 @@ SET COLSEP '|' (goto-char (point-max)) (forward-char -1)) (write-file out-file)))) - (org-table-import out-file (if (string= engine "sqsh") '(4) '(16))) + (org-table-import out-file (if (memq (intern engine) '(athena sqsh)) '(4) '(16))) (org-babel-reassemble-table (mapcar (lambda (x) (if (string= (car x) header-delim) -- 2.39.0 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [patch] ob-clojure: Fix results output 2023-03-14 13:38 ` Daniel Kraus @ 2023-03-14 14:27 ` Daniel Kraus 2023-03-15 10:20 ` Ihor Radchenko 0 siblings, 1 reply; 15+ messages in thread From: Daniel Kraus @ 2023-03-14 14:27 UTC (permalink / raw) To: Ihor Radchenko, emacs-orgmode [-- Attachment #1: Type: text/plain, Size: 221 bytes --] Daniel Kraus <daniel@kraus.my> writes: > Attached is the new patch with the changes. > > [2. text/x-patch; 0001-lisp-ob-sql.el-Add-support-for-Athena.patch]... Ups, I attached the wrong one. Here the correct patch.. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-ob-clojure.el-Fix-results-output-support-clojure-cli.patch --] [-- Type: text/x-patch, Size: 15014 bytes --] From db0634b5ab0b5c8c996c5dcbbeb266b720c67459 Mon Sep 17 00:00:00 2001 From: Daniel Kraus <daniel@kraus.my> Date: Thu, 9 Mar 2023 16:11:27 +0100 Subject: [PATCH] ob-clojure.el: Fix results output, support clojure-cli * lisp/ob-clojure.el (org-babel-clojure-backend): Add support for clojure-cli. * lisp/ob-clojure.el (org-babel-clojurescript-backend): Move nbb to clojurescript. * lisp/ob-clojure.el (org-babel-expand-body:clojure) * lisp/ob-clojure.el (ob-clojure-eval-with-cider): Return only the last expression when :results is not set or value, and return only stdout when :results is set to output. * lisp/ob-clojure.el (ob-clojure-eval-with-cmd): Rename function as it is not only for babashka. * lisp/ob-clojure.el (org-babel-execute:clojure): Differentiate between Clojure and ClojureScript source blocks. The problem was that the ob-clojure results where not correctly taking the results parameter into account. E.g. with the cider backend, you would get all printed or returned values for each line in your block: (def small-map {:a 2 :b 4 :c 8}) {:some :map} (prn :xx) (:b small-map) | #'user/small-map | | {:some :map} | | 4 | or for babashka you would only get the printed values but not the last return value: (def small-map {:a 2 :b 4 :c 8}) {:some :map} (prn :xx) (:b small-map) : :xx Now when you specify :results value, the result is only the last returned value, and with :results output you get all values printed to stdout. So the examples above would all result in the same: (def small-map {:a 2 :b 4 :c 8}) {:some :map} (prn :xx) (:b small-map) : 4 --- etc/ORG-NEWS | 23 +++++++ lisp/ob-clojure.el | 149 ++++++++++++++++++++++++++++----------------- lisp/org-compat.el | 4 ++ 3 files changed, 120 insertions(+), 56 deletions(-) diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS index b9d7b3459..4ca13af17 100644 --- a/etc/ORG-NEWS +++ b/etc/ORG-NEWS @@ -96,6 +96,21 @@ The face ~org-agenda-calendar-daterange~ is used to show entries with a date range in the agenda. It inherits from the default face in order to remain backward-compatible. +*** New ~org-babel-clojurescript-backend~ option to choose ClojureScript backend + +Before, a ClojureScript source block used the same backend as Clojure, +configured in ~org-babel-clojure-backend~ and relied on an undocumented +~:target~ paramter. + +Now, there's ~org-babel-clojurescript-backend~ to determine the +backend used for evaluation of ClojureScript. + +*** Support for Clojure CLI in ~ob-clojure~ + +~ob-clojure~ now supports executing babel source blocks with the +official [[https://clojure.org/guides/deps_and_cli][Clojure CLI tools]]. +The command can be customized with ~ob-clojure-cli-command~. + ** New features *** ~org-metaup~ and ~org-metadown~ now act on headings in region @@ -116,6 +131,14 @@ selection. TODO state, priority, tags, statistics cookies, and COMMENT keywords are allowed in the tree structure. +** Miscellaneous +*** Remove undocumented ~:target~ header parameter in ~ob-clojure~ + +The ~:target~ header was only used internally to distinguish +from Clojure and ClojureScript. +This is now handled with an optional function parameter in +the respective functions that need this information. + * Version 9.6 ** Important announcements and breaking changes diff --git a/lisp/ob-clojure.el b/lisp/ob-clojure.el index 5f589db00..f254fa204 100644 --- a/lisp/ob-clojure.el +++ b/lisp/ob-clojure.el @@ -25,20 +25,21 @@ ;;; Commentary: -;; Support for evaluating Clojure code +;; Support for evaluating Clojure / ClojureScript code. ;; Requirements: ;; - Clojure (at least 1.2.0) ;; - clojure-mode -;; - inf-clojure, Cider, SLIME, babashka or nbb +;; - babashka, nbb, Clojure CLI tools, Cider, inf-clojure or SLIME ;; For clojure-mode, see https://github.com/clojure-emacs/clojure-mode -;; For inf-clojure, see https://github.com/clojure-emacs/inf-clojure -;; For Cider, see https://github.com/clojure-emacs/cider -;; For SLIME, see https://slime.common-lisp.dev ;; For babashka, see https://github.com/babashka/babashka ;; For nbb, see https://github.com/babashka/nbb +;; For Clojure CLI tools, see https://clojure.org/guides/deps_and_cli +;; For Cider, see https://github.com/clojure-emacs/cider +;; For inf-clojure, see https://github.com/clojure-emacs/inf-clojure +;; For SLIME, see https://slime.common-lisp.dev ;; For SLIME, the best way to install its components is by following ;; the directions as set out by Phil Hagelberg (Technomancy) on the @@ -78,20 +79,33 @@ defvar org-babel-header-args:clojurescript (defcustom org-babel-clojure-backend (cond ((executable-find "bb") 'babashka) - ((executable-find "nbb") 'nbb) + ((executable-find "clojure") 'clojure-cli) ((featurep 'cider) 'cider) ((featurep 'inf-clojure) 'inf-clojure) ((featurep 'slime) 'slime) (t nil)) "Backend used to evaluate Clojure code blocks." :group 'org-babel - :package-version '(Org . "9.6") + :package-version '(Org . "9.7") :type '(choice - (const :tag "inf-clojure" inf-clojure) + (const :tag "babashka" babashka) + (const :tag "clojure-cli" clojure-cli) (const :tag "cider" cider) + (const :tag "inf-clojure" inf-clojure) (const :tag "slime" slime) - (const :tag "babashka" babashka) + (const :tag "Not configured yet" nil))) + +(defcustom org-babel-clojurescript-backend + (cond + ((or (executable-find "nbb") (executable-find "npx")) 'nbb) + ((featurep 'cider) 'cider) + (t nil)) + "Backend used to evaluate Clojure code blocks." + :group 'org-babel + :package-version '(Org . "9.7") + :type '(choice (const :tag "nbb" nbb) + (const :tag "cider" cider) (const :tag "Not configured yet" nil))) (defcustom org-babel-clojure-default-ns "user" @@ -105,14 +119,24 @@ defcustom ob-clojure-babashka-command :group 'org-babel :package-version '(Org . "9.6")) -(defcustom ob-clojure-nbb-command (executable-find "nbb") - "Path to the nbb executable." - :type '(choice file (const nil)) +(defcustom ob-clojure-nbb-command (or (executable-find "nbb") + (when-let (npx (executable-find "npx")) + (concat npx " nbb"))) + "Command to invoke the nbb executable." + :type '(choice string (const nil)) :group 'org-babel - :package-version '(Org . "9.6")) + :package-version '(Org . "9.7")) -(defun org-babel-expand-body:clojure (body params) - "Expand BODY according to PARAMS, return the expanded body." +(defcustom ob-clojure-cli-command (when-let (cmd (executable-find "clojure")) + (concat cmd " -M")) + "Clojure CLI command used to execute source blocks." + :type 'string + :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." (let* ((vars (org-babel--get-vars params)) (backend-override (cdr (assq :backend params))) (org-babel-clojure-backend @@ -146,10 +170,26 @@ defun org-babel-expand-body:clojure vars "\n ") body)))))) - (if (or (member "code" result-params) - (member "pp" result-params)) - (format "(clojure.pprint/pprint (do %s))" body) - 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 "))")))) (defvar ob-clojure-inf-clojure-filter-out) (defvar ob-clojure-inf-clojure-tmp-output) @@ -225,32 +265,19 @@ defun ob-clojure-eval-with-inf-clojure s)) (reverse ob-clojure-inf-clojure-tmp-output))))) -(defun ob-clojure-eval-with-cider (expanded params) - "Evaluate EXPANDED code block with PARAMS using cider." +(defun ob-clojure-eval-with-cider (expanded params &optional cljs-p) + "Evaluate EXPANDED code block with PARAMS using cider. +When CLJS-P is non-nil, use a cljs connection instead of clj." (org-require-package 'cider "Cider") - (let ((connection (cider-current-connection (cdr (assq :target params)))) - (result-params (cdr (assq :result-params params))) - result0) + (let ((connection (cider-current-connection (if cljs-p "cljs" "clj")))) (unless connection (sesman-start-session 'CIDER)) (if (not connection) ;; Display in the result instead of using `user-error' - (setq result0 "Please reevaluate when nREPL is connected") - (ob-clojure-with-temp-expanded expanded params - (let ((response (nrepl-sync-request:eval exp connection))) - (push (or (nrepl-dict-get response "root-ex") - (nrepl-dict-get response "ex") - (nrepl-dict-get - response (if (or (member "output" result-params) - (member "pp" result-params)) - "out" - "value"))) - result0))) - (ob-clojure-string-or-list - ;; Filter out s-expressions that return nil (string "nil" - ;; from nrepl eval) or comment forms (actual nil from nrepl) - (reverse (delete "" (mapcar (lambda (r) - (replace-regexp-in-string "nil" "" (or r ""))) - result0))))))) + "Please reevaluate when nREPL is connected" + (let ((response (nrepl-sync-request:eval expanded connection))) + (or (nrepl-dict-get response "root-ex") + (nrepl-dict-get response "ex") + (nrepl-dict-get response "out")))))) (defun ob-clojure-eval-with-slime (expanded params) "Evaluate EXPANDED code block with PARAMS using slime." @@ -262,39 +289,49 @@ defun ob-clojure-eval-with-slime ,(buffer-substring-no-properties (point-min) (point-max))) (cdr (assq :package params))))) -(defun ob-clojure-eval-with-babashka (bb expanded) - "Evaluate EXPANDED code block using BB (babashka or nbb)." - (let ((script-file (org-babel-temp-file "clojure-bb-script-" ".clj"))) +(defun ob-clojure-eval-with-cmd (cmd expanded) + "Evaluate EXPANDED code block using CMD (babashka, clojure or nbb)." + (let ((script-file (org-babel-temp-file "clojure-cmd-script-" ".clj"))) (with-temp-file script-file (insert expanded)) (org-babel-eval - (format "%s %s" bb (org-babel-process-file-name script-file)) + (format "%s %s" cmd (org-babel-process-file-name script-file)) ""))) -(defun org-babel-execute:clojure (body params) - "Execute the BODY block of Clojure code with PARAMS using Babel." +(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 +instead of Clojure." (let* ((backend-override (cdr (assq :backend params))) (org-babel-clojure-backend (cond (backend-override (intern backend-override)) - (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"))))) - (let* ((expanded (org-babel-expand-body:clojure body params)) + (org-babel-clojure-backend (if cljs-p + org-babel-clojurescript-backend + org-babel-clojure-backend)) + (t (user-error "You need to customize `%S' +or set the `:backend' header argument" + (if cljs-p + org-babel-clojurescript-backend + org-babel-clojure-backend)))))) + (let* ((expanded (org-babel-expand-body:clojure body 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)) + ((eq org-babel-clojure-backend 'clojure-cli) + (ob-clojure-eval-with-cmd ob-clojure-cli-command expanded)) ((eq org-babel-clojure-backend 'babashka) - (ob-clojure-eval-with-babashka ob-clojure-babashka-command expanded)) + (ob-clojure-eval-with-cmd ob-clojure-babashka-command expanded)) ((eq org-babel-clojure-backend 'nbb) - (ob-clojure-eval-with-babashka ob-clojure-nbb-command expanded)) + (ob-clojure-eval-with-cmd ob-clojure-nbb-command expanded)) ((eq org-babel-clojure-backend 'cider) - (ob-clojure-eval-with-cider expanded params)) + (ob-clojure-eval-with-cider expanded params cljs-p)) ((eq org-babel-clojure-backend 'slime) - (ob-clojure-eval-with-slime expanded params)))) + (ob-clojure-eval-with-slime expanded params)) + (t (user-error "Invalid backend")))) (org-babel-result-cond result-params result (condition-case nil (org-babel-script-escape result) @@ -302,7 +339,7 @@ defun org-babel-execute:clojure (defun org-babel-execute:clojurescript (body params) "Evaluate BODY with PARAMS as ClojureScript code." - (org-babel-execute:clojure body (cons '(:target . "cljs") params))) + (org-babel-execute:clojure body params t)) (provide 'ob-clojure) diff --git a/lisp/org-compat.el b/lisp/org-compat.el index fadb51df6..c47a4e8c2 100644 --- a/lisp/org-compat.el +++ b/lisp/org-compat.el @@ -71,6 +71,7 @@ (declare-function outline-next-heading "outline" ()) (declare-function speedbar-line-directory "speedbar" (&optional depth)) (declare-function table--at-cell-p "table" (position &optional object at-column)) +(declare-function ob-clojure-eval-with-cmd "ob-clojure" (cmd expanded)) (declare-function org-fold-folded-p "org-fold" (&optional pos spec-or-alias)) (declare-function org-fold-hide-sublevels "org-fold" (levels)) (declare-function org-fold-hide-subtree "org-fold" ()) @@ -1127,6 +1128,9 @@ defconst org-babel-python-mode "Only the built-in Python mode is supported in ob-python now." "9.7") +(define-obsolete-function-alias 'ob-clojure-eval-with-babashka + #'ob-clojure-eval-with-cmd "9.7") + ;;;; Obsolete link types (eval-after-load 'ol -- 2.39.2 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [patch] ob-clojure: Fix results output 2023-03-14 14:27 ` Daniel Kraus @ 2023-03-15 10:20 ` Ihor Radchenko 2023-03-15 11:22 ` Daniel Kraus 0 siblings, 1 reply; 15+ messages in thread From: Ihor Radchenko @ 2023-03-15 10:20 UTC (permalink / raw) To: Daniel Kraus; +Cc: emacs-orgmode Daniel Kraus <daniel@kraus.my> writes: > Ups, I attached the wrong one. > Here the correct patch.. Thanks! Some more comments ;) > (defcustom org-babel-clojure-backend (cond > ((executable-find "bb") 'babashka) > - ((executable-find "nbb") 'nbb) > + ((executable-find "clojure") 'clojure-cli) > ((featurep 'cider) 'cider) > ((featurep 'inf-clojure) 'inf-clojure) > ((featurep 'slime) 'slime) > (t nil)) What will happen with users who customized `org-babel-clojure-backend' to 'nbb in the past? > +(defcustom org-babel-clojurescript-backend > + (cond > + ((or (executable-find "nbb") (executable-find "npx")) 'nbb) > + ((featurep 'cider) 'cider) > + (t nil)) > + "Backend used to evaluate Clojure code blocks." This docstring is exactly the same with `org-babel-clojure-backend'. What is the difference? I think ""Backend used to evaluate ClojureScript code blocks." will be more clear. I feel that other docstrings may also need to be clarified depending whether they affect Clojure or ClojureScript blocks. -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [patch] ob-clojure: Fix results output 2023-03-15 10:20 ` Ihor Radchenko @ 2023-03-15 11:22 ` Daniel Kraus 2023-03-16 10:19 ` Ihor Radchenko 0 siblings, 1 reply; 15+ messages in thread From: Daniel Kraus @ 2023-03-15 11:22 UTC (permalink / raw) To: Ihor Radchenko; +Cc: emacs-orgmode [-- Attachment #1: Type: text/plain, Size: 1008 bytes --] Ihor Radchenko <yantar92@posteo.net> writes: > What will happen with users who customized `org-babel-clojure-backend' > to 'nbb in the past? They would have gotten an error. I changed it now that 'nbb backend is still allowed in a clojure source block but it will internally treated as ClojureScript. >> +(defcustom org-babel-clojurescript-backend >> + (cond >> + ((or (executable-find "nbb") (executable-find "npx")) 'nbb) >> + ((featurep 'cider) 'cider) >> + (t nil)) >> + "Backend used to evaluate Clojure code blocks." > > This docstring is exactly the same with `org-babel-clojure-backend'. > What is the difference? > I think ""Backend used to evaluate ClojureScript code blocks." will be > more clear. I feel that other docstrings may also need to be clarified > depending whether they affect Clojure or ClojureScript blocks. I changed the docstrings to always mention either Clojure or ClojureScript. I'm open for more improvements/suggestions. Attached a new patch. Thanks, Daniel [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-ob-clojure.el-Fix-results-output-support-clojure-cli.patch --] [-- Type: text/x-patch, Size: 15600 bytes --] From 391bdd403f643fa75cceeb0c81f117996c2374b0 Mon Sep 17 00:00:00 2001 From: Daniel Kraus <daniel@kraus.my> Date: Thu, 9 Mar 2023 16:11:27 +0100 Subject: [PATCH] ob-clojure.el: Fix results output, support clojure-cli * lisp/ob-clojure.el (org-babel-clojure-backend): Add support for clojure-cli. * lisp/ob-clojure.el (org-babel-clojurescript-backend): Move nbb to clojurescript. * lisp/ob-clojure.el (org-babel-expand-body:clojure) * lisp/ob-clojure.el (ob-clojure-eval-with-cider): Return only the last expression when :results is not set or value, and return only stdout when :results is set to output. * lisp/ob-clojure.el (ob-clojure-eval-with-cmd): Rename function as it is not only for babashka. * lisp/ob-clojure.el (org-babel-execute:clojure): Differentiate between Clojure and ClojureScript source blocks. The problem was that the ob-clojure results where not correctly taking the results parameter into account. E.g. with the cider backend, you would get all printed or returned values for each line in your block: (def small-map {:a 2 :b 4 :c 8}) {:some :map} (prn :xx) (:b small-map) | #'user/small-map | | {:some :map} | | 4 | or for babashka you would only get the printed values but not the last return value: (def small-map {:a 2 :b 4 :c 8}) {:some :map} (prn :xx) (:b small-map) : :xx Now when you specify :results value, the result is only the last returned value, and with :results output you get all values printed to stdout. So the examples above would all result in the same: (def small-map {:a 2 :b 4 :c 8}) {:some :map} (prn :xx) (:b small-map) : 4 --- etc/ORG-NEWS | 23 +++++++ lisp/ob-clojure.el | 156 ++++++++++++++++++++++++++++----------------- lisp/org-compat.el | 4 ++ 3 files changed, 126 insertions(+), 57 deletions(-) diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS index b9d7b3459..4ca13af17 100644 --- a/etc/ORG-NEWS +++ b/etc/ORG-NEWS @@ -96,6 +96,21 @@ The face ~org-agenda-calendar-daterange~ is used to show entries with a date range in the agenda. It inherits from the default face in order to remain backward-compatible. +*** New ~org-babel-clojurescript-backend~ option to choose ClojureScript backend + +Before, a ClojureScript source block used the same backend as Clojure, +configured in ~org-babel-clojure-backend~ and relied on an undocumented +~:target~ paramter. + +Now, there's ~org-babel-clojurescript-backend~ to determine the +backend used for evaluation of ClojureScript. + +*** Support for Clojure CLI in ~ob-clojure~ + +~ob-clojure~ now supports executing babel source blocks with the +official [[https://clojure.org/guides/deps_and_cli][Clojure CLI tools]]. +The command can be customized with ~ob-clojure-cli-command~. + ** New features *** ~org-metaup~ and ~org-metadown~ now act on headings in region @@ -116,6 +131,14 @@ selection. TODO state, priority, tags, statistics cookies, and COMMENT keywords are allowed in the tree structure. +** Miscellaneous +*** Remove undocumented ~:target~ header parameter in ~ob-clojure~ + +The ~:target~ header was only used internally to distinguish +from Clojure and ClojureScript. +This is now handled with an optional function parameter in +the respective functions that need this information. + * Version 9.6 ** Important announcements and breaking changes diff --git a/lisp/ob-clojure.el b/lisp/ob-clojure.el index 5f589db00..70e032154 100644 --- a/lisp/ob-clojure.el +++ b/lisp/ob-clojure.el @@ -25,20 +25,21 @@ ;;; Commentary: -;; Support for evaluating Clojure code +;; Support for evaluating Clojure / ClojureScript code. ;; Requirements: ;; - Clojure (at least 1.2.0) ;; - clojure-mode -;; - inf-clojure, Cider, SLIME, babashka or nbb +;; - babashka, nbb, Clojure CLI tools, Cider, inf-clojure or SLIME ;; For clojure-mode, see https://github.com/clojure-emacs/clojure-mode -;; For inf-clojure, see https://github.com/clojure-emacs/inf-clojure -;; For Cider, see https://github.com/clojure-emacs/cider -;; For SLIME, see https://slime.common-lisp.dev ;; For babashka, see https://github.com/babashka/babashka ;; For nbb, see https://github.com/babashka/nbb +;; For Clojure CLI tools, see https://clojure.org/guides/deps_and_cli +;; For Cider, see https://github.com/clojure-emacs/cider +;; For inf-clojure, see https://github.com/clojure-emacs/inf-clojure +;; For SLIME, see https://slime.common-lisp.dev ;; For SLIME, the best way to install its components is by following ;; the directions as set out by Phil Hagelberg (Technomancy) on the @@ -78,20 +79,33 @@ defvar org-babel-header-args:clojurescript (defcustom org-babel-clojure-backend (cond ((executable-find "bb") 'babashka) - ((executable-find "nbb") 'nbb) + ((executable-find "clojure") 'clojure-cli) ((featurep 'cider) 'cider) ((featurep 'inf-clojure) 'inf-clojure) ((featurep 'slime) 'slime) (t nil)) "Backend used to evaluate Clojure code blocks." :group 'org-babel - :package-version '(Org . "9.6") + :package-version '(Org . "9.7") :type '(choice - (const :tag "inf-clojure" inf-clojure) + (const :tag "babashka" babashka) + (const :tag "clojure-cli" clojure-cli) (const :tag "cider" cider) + (const :tag "inf-clojure" inf-clojure) (const :tag "slime" slime) - (const :tag "babashka" babashka) + (const :tag "Not configured yet" nil))) + +(defcustom org-babel-clojurescript-backend + (cond + ((or (executable-find "nbb") (executable-find "npx")) 'nbb) + ((featurep 'cider) 'cider) + (t nil)) + "Backend used to evaluate ClojureScript code blocks." + :group 'org-babel + :package-version '(Org . "9.7") + :type '(choice (const :tag "nbb" nbb) + (const :tag "cider" cider) (const :tag "Not configured yet" nil))) (defcustom org-babel-clojure-default-ns "user" @@ -100,19 +114,29 @@ defcustom org-babel-clojure-default-ns :group 'org-babel) (defcustom ob-clojure-babashka-command (executable-find "bb") - "Path to the babashka executable." + "Babashka command used by the Clojure `babashka' backend." :type '(choice file (const nil)) :group 'org-babel :package-version '(Org . "9.6")) -(defcustom ob-clojure-nbb-command (executable-find "nbb") - "Path to the nbb executable." - :type '(choice file (const nil)) +(defcustom ob-clojure-nbb-command (or (executable-find "nbb") + (when-let (npx (executable-find "npx")) + (concat npx " nbb"))) + "Nbb command used by the ClojureScript `nbb' backend." + :type '(choice string (const nil)) :group 'org-babel - :package-version '(Org . "9.6")) + :package-version '(Org . "9.7")) -(defun org-babel-expand-body:clojure (body params) - "Expand BODY according to PARAMS, return the expanded body." +(defcustom ob-clojure-cli-command (when-let (cmd (executable-find "clojure")) + (concat cmd " -M")) + "Clojure CLI command used by the Clojure `clojure-cli' backend." + :type 'string + :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." (let* ((vars (org-babel--get-vars params)) (backend-override (cdr (assq :backend params))) (org-babel-clojure-backend @@ -146,10 +170,26 @@ defun org-babel-expand-body:clojure vars "\n ") body)))))) - (if (or (member "code" result-params) - (member "pp" result-params)) - (format "(clojure.pprint/pprint (do %s))" body) - 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 "))")))) (defvar ob-clojure-inf-clojure-filter-out) (defvar ob-clojure-inf-clojure-tmp-output) @@ -225,32 +265,19 @@ defun ob-clojure-eval-with-inf-clojure s)) (reverse ob-clojure-inf-clojure-tmp-output))))) -(defun ob-clojure-eval-with-cider (expanded params) - "Evaluate EXPANDED code block with PARAMS using cider." +(defun ob-clojure-eval-with-cider (expanded params &optional cljs-p) + "Evaluate EXPANDED code block with PARAMS using cider. +When CLJS-P is non-nil, use a cljs connection instead of clj." (org-require-package 'cider "Cider") - (let ((connection (cider-current-connection (cdr (assq :target params)))) - (result-params (cdr (assq :result-params params))) - result0) + (let ((connection (cider-current-connection (if cljs-p "cljs" "clj")))) (unless connection (sesman-start-session 'CIDER)) (if (not connection) ;; Display in the result instead of using `user-error' - (setq result0 "Please reevaluate when nREPL is connected") - (ob-clojure-with-temp-expanded expanded params - (let ((response (nrepl-sync-request:eval exp connection))) - (push (or (nrepl-dict-get response "root-ex") - (nrepl-dict-get response "ex") - (nrepl-dict-get - response (if (or (member "output" result-params) - (member "pp" result-params)) - "out" - "value"))) - result0))) - (ob-clojure-string-or-list - ;; Filter out s-expressions that return nil (string "nil" - ;; from nrepl eval) or comment forms (actual nil from nrepl) - (reverse (delete "" (mapcar (lambda (r) - (replace-regexp-in-string "nil" "" (or r ""))) - result0))))))) + "Please reevaluate when nREPL is connected" + (let ((response (nrepl-sync-request:eval expanded connection))) + (or (nrepl-dict-get response "root-ex") + (nrepl-dict-get response "ex") + (nrepl-dict-get response "out")))))) (defun ob-clojure-eval-with-slime (expanded params) "Evaluate EXPANDED code block with PARAMS using slime." @@ -262,39 +289,54 @@ defun ob-clojure-eval-with-slime ,(buffer-substring-no-properties (point-min) (point-max))) (cdr (assq :package params))))) -(defun ob-clojure-eval-with-babashka (bb expanded) - "Evaluate EXPANDED code block using BB (babashka or nbb)." - (let ((script-file (org-babel-temp-file "clojure-bb-script-" ".clj"))) +(defun ob-clojure-eval-with-cmd (cmd expanded) + "Evaluate EXPANDED code block using CMD (babashka, clojure or nbb)." + (let ((script-file (org-babel-temp-file "clojure-cmd-script-" ".clj"))) (with-temp-file script-file (insert expanded)) (org-babel-eval - (format "%s %s" bb (org-babel-process-file-name script-file)) + (format "%s %s" cmd (org-babel-process-file-name script-file)) ""))) -(defun org-babel-execute:clojure (body params) - "Execute the BODY block of Clojure code with PARAMS using Babel." +(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 +instead of Clojure." (let* ((backend-override (cdr (assq :backend params))) (org-babel-clojure-backend (cond (backend-override (intern backend-override)) - (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"))))) - (let* ((expanded (org-babel-expand-body:clojure body params)) + (org-babel-clojure-backend (if cljs-p + org-babel-clojurescript-backend + org-babel-clojure-backend)) + (t (user-error "You need to customize `%S' +or set the `:backend' header argument" + (if cljs-p + org-babel-clojurescript-backend + org-babel-clojure-backend))))) + ;; We allow a Clojure source block to be evaluated with the + ;; nbb backend and therefore have to expand the body with + ;; 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)) (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)) + ((eq org-babel-clojure-backend 'clojure-cli) + (ob-clojure-eval-with-cmd ob-clojure-cli-command expanded)) ((eq org-babel-clojure-backend 'babashka) - (ob-clojure-eval-with-babashka ob-clojure-babashka-command expanded)) + (ob-clojure-eval-with-cmd ob-clojure-babashka-command expanded)) ((eq org-babel-clojure-backend 'nbb) - (ob-clojure-eval-with-babashka ob-clojure-nbb-command expanded)) + (ob-clojure-eval-with-cmd ob-clojure-nbb-command expanded)) ((eq org-babel-clojure-backend 'cider) - (ob-clojure-eval-with-cider expanded params)) + (ob-clojure-eval-with-cider expanded params cljs-p)) ((eq org-babel-clojure-backend 'slime) - (ob-clojure-eval-with-slime expanded params)))) + (ob-clojure-eval-with-slime expanded params)) + (t (user-error "Invalid backend")))) (org-babel-result-cond result-params result (condition-case nil (org-babel-script-escape result) @@ -302,7 +344,7 @@ defun org-babel-execute:clojure (defun org-babel-execute:clojurescript (body params) "Evaluate BODY with PARAMS as ClojureScript code." - (org-babel-execute:clojure body (cons '(:target . "cljs") params))) + (org-babel-execute:clojure body params t)) (provide 'ob-clojure) diff --git a/lisp/org-compat.el b/lisp/org-compat.el index fadb51df6..c47a4e8c2 100644 --- a/lisp/org-compat.el +++ b/lisp/org-compat.el @@ -71,6 +71,7 @@ (declare-function outline-next-heading "outline" ()) (declare-function speedbar-line-directory "speedbar" (&optional depth)) (declare-function table--at-cell-p "table" (position &optional object at-column)) +(declare-function ob-clojure-eval-with-cmd "ob-clojure" (cmd expanded)) (declare-function org-fold-folded-p "org-fold" (&optional pos spec-or-alias)) (declare-function org-fold-hide-sublevels "org-fold" (levels)) (declare-function org-fold-hide-subtree "org-fold" ()) @@ -1127,6 +1128,9 @@ defconst org-babel-python-mode "Only the built-in Python mode is supported in ob-python now." "9.7") +(define-obsolete-function-alias 'ob-clojure-eval-with-babashka + #'ob-clojure-eval-with-cmd "9.7") + ;;;; Obsolete link types (eval-after-load 'ol -- 2.39.2 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [patch] ob-clojure: Fix results output 2023-03-15 11:22 ` Daniel Kraus @ 2023-03-16 10:19 ` Ihor Radchenko 2023-03-19 8:07 ` Ihor Radchenko 0 siblings, 1 reply; 15+ messages in thread From: Ihor Radchenko @ 2023-03-16 10:19 UTC (permalink / raw) To: Daniel Kraus; +Cc: emacs-orgmode Daniel Kraus <daniel@kraus.my> writes: > Attached a new patch. Thanks! I have no further comments. Feel free to push. -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [patch] ob-clojure: Fix results output 2023-03-16 10:19 ` Ihor Radchenko @ 2023-03-19 8:07 ` Ihor Radchenko 2023-03-19 20:43 ` Daniel Kraus 0 siblings, 1 reply; 15+ messages in thread From: Ihor Radchenko @ 2023-03-19 8:07 UTC (permalink / raw) To: Daniel Kraus; +Cc: emacs-orgmode Ihor Radchenko <yantar92@posteo.net> writes: > Feel free to push. I note that we now have a new compiler warning after your changes: ⛔ Warning (comp): ob-clojure.el:268:45: Warning: Unused lexical argument ‘params’ May you please take a look? If the function argument is really unused, replace it with _ in `ob-clojure-eval-with-cider'. -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [patch] ob-clojure: Fix results output 2023-03-19 8:07 ` Ihor Radchenko @ 2023-03-19 20:43 ` Daniel Kraus 2023-03-22 10:48 ` Ihor Radchenko 0 siblings, 1 reply; 15+ messages in thread From: Daniel Kraus @ 2023-03-19 20:43 UTC (permalink / raw) To: Ihor Radchenko; +Cc: emacs-orgmode Ihor Radchenko <yantar92@posteo.net> writes: > I note that we now have a new compiler warning after your changes: > ⛔ Warning (comp): ob-clojure.el:268:45: Warning: Unused lexical argument ‘params’ > > May you please take a look? > If the function argument is really unused, replace it with _ in > `ob-clojure-eval-with-cider'. Ups, sorry. Before `params` was only used to receive the :target parameter if it's a cljs or clj block. But that's now just a regular parameter to the function. I fixed it with a _ prefix. Cheers, Daniel ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [patch] ob-clojure: Fix results output 2023-03-19 20:43 ` Daniel Kraus @ 2023-03-22 10:48 ` Ihor Radchenko 2023-03-23 11:31 ` Daniel Kraus 0 siblings, 1 reply; 15+ messages in thread From: Ihor Radchenko @ 2023-03-22 10:48 UTC (permalink / raw) To: Daniel Kraus; +Cc: emacs-orgmode Daniel Kraus <daniel@kraus.my> writes: > Ihor Radchenko <yantar92@posteo.net> writes: > >> I note that we now have a new compiler warning after your changes: >> ⛔ Warning (comp): ob-clojure.el:268:45: Warning: Unused lexical argument ‘params’ >> >> May you please take a look? >> If the function argument is really unused, replace it with _ in >> `ob-clojure-eval-with-cider'. > > Ups, sorry. > Before `params` was only used to receive the :target parameter if it's a cljs or clj > block. But that's now just a regular parameter to the function. > I fixed it with a _ prefix. Now, the docstring appears to be a bit confusing: (defun ob-clojure-eval-with-cider (expanded _params &optional cljs-p) "Evaluate EXPANDED code block with PARAMS using cider. When CLJS-P is non-nil, use a cljs connection instead of clj." It would be useful to mention that PARAMS argument is unused. -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [patch] ob-clojure: Fix results output 2023-03-22 10:48 ` Ihor Radchenko @ 2023-03-23 11:31 ` Daniel Kraus 2023-03-23 11:52 ` Ihor Radchenko 0 siblings, 1 reply; 15+ messages in thread From: Daniel Kraus @ 2023-03-23 11:31 UTC (permalink / raw) To: Ihor Radchenko; +Cc: emacs-orgmode Ihor Radchenko <yantar92@posteo.net> writes: > Now, the docstring appears to be a bit confusing: > > (defun ob-clojure-eval-with-cider (expanded _params &optional cljs-p) > "Evaluate EXPANDED code block with PARAMS using cider. > When CLJS-P is non-nil, use a cljs connection instead of clj." > > It would be useful to mention that PARAMS argument is unused. Should I go with your initial suggestion and just replace it with _? Like (defun ob-clojure-eval-with-cider (expanded _ &optional cljs-p) "Evaluate EXPANDED code block using cider. When CLJS-P is non-nil, use a cljs connection instead of clj." But then someone will maybe wonder why there is unused argument? Or rather something like: (defun ob-clojure-eval-with-cider (expanded _params &optional cljs-p) "Evaluate EXPANDED code block using cider. When CLJS-P is non-nil, use a cljs connection instead of clj. The PARAMS from Babel are not used in this function." Writing good docstrings is hard :D Cheers, Daniel ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [patch] ob-clojure: Fix results output 2023-03-23 11:31 ` Daniel Kraus @ 2023-03-23 11:52 ` Ihor Radchenko 2023-03-23 12:29 ` Daniel Kraus 0 siblings, 1 reply; 15+ messages in thread From: Ihor Radchenko @ 2023-03-23 11:52 UTC (permalink / raw) To: Daniel Kraus; +Cc: emacs-orgmode Daniel Kraus <daniel@kraus.my> writes: > Ihor Radchenko <yantar92@posteo.net> writes: > >> Now, the docstring appears to be a bit confusing: >> >> (defun ob-clojure-eval-with-cider (expanded _params &optional cljs-p) >> "Evaluate EXPANDED code block with PARAMS using cider. >> When CLJS-P is non-nil, use a cljs connection instead of clj." >> >> It would be useful to mention that PARAMS argument is unused. > > Should I go with your initial suggestion and just replace it with _? > Like > > (defun ob-clojure-eval-with-cider (expanded _ &optional cljs-p) > "Evaluate EXPANDED code block using cider. > When CLJS-P is non-nil, use a cljs connection instead of clj." > > But then someone will maybe wonder why there is unused argument? > > Or rather something like: > > (defun ob-clojure-eval-with-cider (expanded _params &optional cljs-p) > "Evaluate EXPANDED code block using cider. > When CLJS-P is non-nil, use a cljs connection instead of clj. > The PARAMS from Babel are not used in this function." I like the second variant better. -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [patch] ob-clojure: Fix results output 2023-03-23 11:52 ` Ihor Radchenko @ 2023-03-23 12:29 ` Daniel Kraus 0 siblings, 0 replies; 15+ messages in thread From: Daniel Kraus @ 2023-03-23 12:29 UTC (permalink / raw) To: Ihor Radchenko; +Cc: emacs-orgmode Ihor Radchenko <yantar92@posteo.net> writes: >> Or rather something like: >> >> (defun ob-clojure-eval-with-cider (expanded _params &optional cljs-p) >> "Evaluate EXPANDED code block using cider. >> When CLJS-P is non-nil, use a cljs connection instead of clj. >> The PARAMS from Babel are not used in this function." > > I like the second variant better. Thanks. I pushed the fix. Cheers, Daniel ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2023-03-23 12:31 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-03-09 15:40 [patch] ob-clojure: Fix results output Daniel Kraus 2023-03-10 12:35 ` Ihor Radchenko 2023-03-13 14:01 ` Daniel Kraus 2023-03-14 12:35 ` Ihor Radchenko 2023-03-14 13:38 ` Daniel Kraus 2023-03-14 14:27 ` Daniel Kraus 2023-03-15 10:20 ` Ihor Radchenko 2023-03-15 11:22 ` Daniel Kraus 2023-03-16 10:19 ` Ihor Radchenko 2023-03-19 8:07 ` Ihor Radchenko 2023-03-19 20:43 ` Daniel Kraus 2023-03-22 10:48 ` Ihor Radchenko 2023-03-23 11:31 ` Daniel Kraus 2023-03-23 11:52 ` Ihor Radchenko 2023-03-23 12:29 ` Daniel Kraus
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).