emacs-orgmode
[Top][All Lists]
Advanced

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

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


From: Ihor Radchenko
Subject: Re: [patch] ob-clojure: Fix results output
Date: Fri, 10 Mar 2023 12:35:10 +0000

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>



reply via email to

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