Re: [patch] ob-clojure: Fix results output

[Daniel Kraus]

Ihor Radchenko  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

Re: [patch] ob-clojure: Fix results output

[Ihor Radchenko]

Daniel Kraus  writes:

> Ihor Radchenko  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 .
Support Org development at ,
or support my work at 

Re: [patch] ob-clojure: Fix results output

[Daniel Kraus]

Ihor Radchenko  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

Re: [patch] ob-clojure: Fix results output

[Ihor Radchenko]

Daniel Kraus  writes:

> Ihor Radchenko  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 .
Support Org development at ,
or support my work at 

Re: [patch] ob-clojure: Fix results output

[Daniel Kraus]

Ihor Radchenko  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

Re: [patch] ob-clojure: Fix results output

[Ihor Radchenko]

Ihor Radchenko  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 .
Support Org development at ,
or support my work at 

Re: [patch] ob-clojure: Fix results output

[Ihor Radchenko]

Daniel Kraus  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 .
Support Org development at ,
or support my work at 

Re: [patch] ob-clojure: Fix results output

[Daniel Kraus]

Ihor Radchenko  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

>From 391bdd403f643fa75cceeb0c81f117996c2374b0 Mon Sep 17 00:00:00 2001
From: Daniel Kraus 
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
 ;;

Re: [patch] ob-clojure: Fix results output

[Ihor Radchenko]

Daniel Kraus  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 .
Support Org development at ,
or support my work at 

Re: [patch] ob-clojure: Fix results output

[Daniel Kraus]

Daniel Kraus  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..


>From db0634b5ab0b5c8c996c5dcbbeb266b720c67459 Mon Sep 17 00:00:00 2001
From: Daniel Kraus 
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)
+   

Re: [patch] ob-clojure: Fix results output

[Daniel Kraus]

Hi!

Ihor Radchenko  writes:

> Daniel Kraus  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
>From ddace051205d20b24c047962ca9d1335bdd90284 Mon Sep 17 00:00:00 2001
From: Daniel Kraus 
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

Re: [patch] ob-clojure: Fix results output

[Ihor Radchenko]

Daniel Kraus  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 .
Support Org development at ,
or support my work at 

Re: [patch] ob-clojure: Fix results output

[Daniel Kraus]

Hi!

Ihor Radchenko  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
>From 3ad98fa88f6ebd4ae1b2b41d66cca9e9d6e1875e Mon Sep 17 00:00:00 2001
From: Daniel Kraus 
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 an

Re: [patch] ob-clojure: Fix results output

[Ihor Radchenko]

Daniel Kraus  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 .
Support Org development at ,
or support my work at