emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
From: Ihor Radchenko <yantar92@posteo.net>
To: Daniel Kraus <daniel@kraus.my>
Cc: emacs-orgmode@gnu.org
Subject: Re: [patch] ob-clojure: Fix results output
Date: Fri, 10 Mar 2023 12:35:10 +0000	[thread overview]
Message-ID: <87pm9gyebl.fsf@localhost> (raw)
In-Reply-To: <87mt4m53qj.fsf@kraus.my>

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	other threads:[~2023-03-10 12:34 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-09 15:40 [patch] ob-clojure: Fix results output Daniel Kraus
2023-03-10 12:35 ` Ihor Radchenko [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.orgmode.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87pm9gyebl.fsf@localhost \
    --to=yantar92@posteo.net \
    --cc=daniel@kraus.my \
    --cc=emacs-orgmode@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).