[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [patch] ob-clojure: Fix results output
From: |
Daniel Kraus |
Subject: |
Re: [patch] ob-clojure: Fix results output |
Date: |
Mon, 13 Mar 2023 15:01:11 +0100 |
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
0001-ob-clojure.el-Fix-results-output-support-clojure-cli.patch
Description: Text Data
- [patch] ob-clojure: Fix results output, Daniel Kraus, 2023/03/09
- Re: [patch] ob-clojure: Fix results output, Ihor Radchenko, 2023/03/10
- Re: [patch] ob-clojure: Fix results output,
Daniel Kraus <=
- Re: [patch] ob-clojure: Fix results output, Ihor Radchenko, 2023/03/14
- Re: [patch] ob-clojure: Fix results output, Daniel Kraus, 2023/03/14
- Re: [patch] ob-clojure: Fix results output, Daniel Kraus, 2023/03/14
- Re: [patch] ob-clojure: Fix results output, Ihor Radchenko, 2023/03/15
- Re: [patch] ob-clojure: Fix results output, Daniel Kraus, 2023/03/15
- Re: [patch] ob-clojure: Fix results output, Ihor Radchenko, 2023/03/16
- Re: [patch] ob-clojure: Fix results output, Ihor Radchenko, 2023/03/19
- Re: [patch] ob-clojure: Fix results output, Daniel Kraus, 2023/03/19
- Re: [patch] ob-clojure: Fix results output, Ihor Radchenko, 2023/03/22
- Re: [patch] ob-clojure: Fix results output, Daniel Kraus, 2023/03/23