Re: [PATCH] update ob-scheme to latest changes in geiser package
Tim Van den Langenbergh writes: > I have updated the patch and enclosed it, feel free to drop a line if you > notice anything else obviously missing. Thanks! Applied onto main as 06d08e6f3. Best, Ihor
Re: [PATCH] update ob-scheme to latest changes in geiser package
Ihor Radchenko writes: > > Thanks! Since Felipe has no objections, your patch should be good to apply. > > Could you kindly add the geiser version info as a comment inside code? > Commit message is an unlikely place to be checked regularly and be > noticed as geiser is updated in future. > > Comments are generally preferred over commit messages: > > https://orgmode.org/worg/org-contribute.html#commit-messages -> > -> https://git.savannah.gnu.org/cgit/emacs.git/plain/CONTRIBUTE >>> - Explaining the rationale for a design choice is best done in comments >>> in the source code. However, sometimes it is useful to describe just >>> the rationale for a change; that can be done in the commit message >>> between the summary line and the file entries. > > Best, > Ihor Hello Ihor, thank you for pointing that out, seems I botched my reading comprehension skill check. I have updated the patch and enclosed it, feel free to drop a line if you notice anything else obviously missing. Vale, - Tim From 3b4120d5b3fb41d4b0a5bbdee4cc7a3f4cd02642 Mon Sep 17 00:00:00 2001 From: Tim Van den Langenbergh Date: Sat, 23 Apr 2022 14:57:27 +0200 Subject: [PATCH] lisp/ob-scheme.el: Use synchronous eval. * ob-scheme.el (org-babel-scheme-execute-with-geiser): If available, use the synchronous `geiser-eval-region/wait' function to get the evaluation result. Works with geiser 0.18 (before introduction of async evaluation) and geiser 0.22 (which introduces `geiser-eval-region/wait'). Modified from a patch proposal by Felipe Lema. TINYCHANGE --- lisp/ob-scheme.el | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/lisp/ob-scheme.el b/lisp/ob-scheme.el index 72d2e029e..b91c368a7 100644 --- a/lisp/ob-scheme.el +++ b/lisp/ob-scheme.el @@ -55,6 +55,8 @@ (declare-function geiser-mode "ext:geiser-mode" ()) (declare-function geiser-eval-region "ext:geiser-mode" (start end &optional and-go raw nomsg)) +(declare-function geiser-eval-region/wait "ext:geiser-mode" + (start end &optional timeout)) (declare-function geiser-repl-exit "ext:geiser-repl" (&optional arg)) (declare-function geiser-eval--retort-output "ext:geiser-eval" (ret)) (declare-function geiser-eval--retort-result-str "ext:geiser-eval" (ret prefix)) @@ -176,7 +178,13 @@ is true; otherwise returns the last value." (setq geiser-impl--implementation nil) (let ((geiser-debug-jump-to-debug-p nil) (geiser-debug-show-debug-p nil)) - (let ((ret (geiser-eval-region (point-min) (point-max +;; `geiser-eval-region/wait' was introduced to await the +;; result of async evaluation in geiser version 0.22. + (let ((ret (funcall (if (fboundp 'geiser-eval-region/wait) +#'geiser-eval-region/wait + #'geiser-eval-region) +(point-min) +(point-max (setq result (if output (or (geiser-eval--retort-output ret) "Geiser Interpreter produced no output") -- 2.36.0
Re: [PATCH] update ob-scheme to latest changes in geiser package
Tim Van den Langenbergh writes: > I have taken the liberty of reformatting the patch you created for ob-scheme. > You can find it enclosed. Thanks! Since Felipe has no objections, your patch should be good to apply. > If it is fine with you and the org-mode maintainers it can be applied as-is, > particularly since it is a tiny change. > I have also added some additional information to the commit message, such as > when the `geiser-eval-region/wait' function was introduced. Could you kindly add the geiser version info as a comment inside code? Commit message is an unlikely place to be checked regularly and be noticed as geiser is updated in future. Comments are generally preferred over commit messages: https://orgmode.org/worg/org-contribute.html#commit-messages -> -> https://git.savannah.gnu.org/cgit/emacs.git/plain/CONTRIBUTE >> - Explaining the rationale for a design choice is best done in comments >> in the source code. However, sometimes it is useful to describe just >> the rationale for a change; that can be done in the commit message >> between the summary line and the file entries. Best, Ihor
Re: [PATCH] update ob-scheme to latest changes in geiser package
I'm Ok with this patch. Haven't tested it myself, but looks good otherwise (probably Tim has been using it for quite a while now) Felipe On 2022-04-23 09:29, Tim Van den Langenbergh wrote: Felipe, I have taken the liberty of reformatting the patch you created for ob-scheme. You can find it enclosed. If it is fine with you and the org-mode maintainers it can be applied as-is, particularly since it is a tiny change. I have also added some additional information to the commit message, such as when the `geiser-eval-region/wait' function was introduced. Please feel free to contact me in regards to any comments or questions you may have. Vale, - Tim Van den Langenbergh
Re: [PATCH] update ob-scheme to latest changes in geiser package
Felipe, I have taken the liberty of reformatting the patch you created for ob-scheme. You can find it enclosed. If it is fine with you and the org-mode maintainers it can be applied as-is, particularly since it is a tiny change. I have also added some additional information to the commit message, such as when the `geiser-eval-region/wait' function was introduced. Please feel free to contact me in regards to any comments or questions you may have. Vale, - Tim Van den Langenbergh From d06f6d5b7342693b4065c0c361e430c4bdb065e5 Mon Sep 17 00:00:00 2001 From: Tim Van den Langenbergh Date: Sat, 23 Apr 2022 14:57:27 +0200 Subject: [PATCH] lisp/ob-scheme.el: Use synchronous eval. * ob-scheme.el (org-babel-scheme-execute-with-geiser): If available, use the synchronous `geiser-eval-region/wait' function to get the evaluation result. Works with geiser 0.18 (before introduction of async evaluation) and geiser 0.22 (which introduces `geiser-eval-region/wait'). Modified from a patch proposal by Felipe Lema. TINYCHANGE --- lisp/ob-scheme.el | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lisp/ob-scheme.el b/lisp/ob-scheme.el index 72d2e029e..f28cadd87 100644 --- a/lisp/ob-scheme.el +++ b/lisp/ob-scheme.el @@ -55,6 +55,8 @@ (declare-function geiser-mode "ext:geiser-mode" ()) (declare-function geiser-eval-region "ext:geiser-mode" (start end &optional and-go raw nomsg)) +(declare-function geiser-eval-region/wait "ext:geiser-mode" + (start end &optional timeout)) (declare-function geiser-repl-exit "ext:geiser-repl" (&optional arg)) (declare-function geiser-eval--retort-output "ext:geiser-eval" (ret)) (declare-function geiser-eval--retort-result-str "ext:geiser-eval" (ret prefix)) @@ -176,7 +178,11 @@ is true; otherwise returns the last value." (setq geiser-impl--implementation nil) (let ((geiser-debug-jump-to-debug-p nil) (geiser-debug-show-debug-p nil)) - (let ((ret (geiser-eval-region (point-min) (point-max + (let ((ret (funcall (if (fboundp 'geiser-eval-region/wait) +#'geiser-eval-region/wait + #'geiser-eval-region) +(point-min) +(point-max (setq result (if output (or (geiser-eval--retort-output ret) "Geiser Interpreter produced no output") -- 2.35.3
Re: [PATCH] update ob-scheme to latest changes in geiser package
The patch appears to solve the real issue others already experience. See https://list.orgmode.org/20220205221049.ymkzevpc2ivlxwbc@bob-mtu/T/#t Dear Felipe, Could you kindly update the patch as requested by Max? Best, Ihor
Re: [PATCH] update ob-scheme to latest changes in geiser package
On 31/12/2021 02:01, Felipe Lema wrote: On Thursday, 30 December 2021 22:50:20 +07 Max Nikulin wrote: I would consider testing if `geiser-eval-region/wait' is bound and fallback to `geiser-eval-region' otherwise. I've wrapped an `if` to fallback to previous API before this breaking change in attached patch. Felipe, thanks for the patch. It is tracked on https://updates.orgmode.org Sometimes maintainers merge patches in large batches before a release, so it may takes months. --- a/lisp/org/ob-scheme.el +++ b/lisp/org/ob-scheme.el Almost certainly it should be "lisp/ob-scheme.el" without "org" in the middle. It is better to prepare patches using "git format-patch" since it allows to quickly create a commit with proper message and author using "git am" command. See https://orgmode.org/worg/org-contribute.html for details. + ;; use `geiser-eval-region/wait' only when available + ;; in newer versions of `geiser' Maybe specifying the version where new feature has been introduced would be helpful to maintainers to decide at which moment fallback code path can be removed. I have not tested if geiser works, but a comment to https://gitlab.com/emacs-geiser/geiser/-/issues/30 confirms it. It seems maintainers should check and update your status in respect to FSF papers on https://orgmode.org/worg/contributors.html since earlier you wrote that the procedure had been completed.
Re: [PATCH] update ob-scheme to latest changes in geiser package
On Thursday, 30 December 2021 22:50:20 +07 Max Nikulin wrote: > On Wednesday, 29 December 2021 22.53.18 -03 Felipe Lema wrote: > > On Wednesday, 29 December 2021 15.03.47 -03 Felipe Lema wrote: > > > Sup, y'all > > > > > > The geiser package had a recent update in which `geiser-eval-region` > > > behaves like an async function and does not return evaluation result. In > > > exchange, the newly introduced `geiser-eval-region/wait` does that now. > > > > > > I've attached a patch to update ob-scheme.el to reflect these changes. > > > Without them, evaluating scheme dialect code blocks in Org will end up > > > with empty results. > > > > > > To learn more about the change (and how it impacted other people's flows) > > > see issue thread below > > > > > > https://gitlab.com/emacs-geiser/geiser/-/issues/30 > > > > > > Felipe > > > > > > > > > > I was pointed out that I was missing the corresponding `declare-function`. > > > > I've included it in the attached patch. Sorry about the double-posting > > > > Felipe > > > > I am not a geiser user, so maybe I missed something. Does this change mean > that org becomes incompatible with older geiser versions? E,g, debian and > ubuntu have elpa-geiser system package. > > > I would consider testing if `geiser-eval-region/wait' is bound and fallback > to `geiser-eval-region' otherwise. I've wrapped an `if` to fallback to previous API before this breaking change in attached patch. Felipe update to new API call to evaluate region and wait for result * ob-scheme.el replace `geiser-eval-region` with `geiser-eval-region/wait` (only available in newer versions) as the former changed expected behaviour; the latter returns the result of evaluating. diff --git a/lisp/org/ob-scheme.el b/lisp/org/ob-scheme.el index f4836b23fe..ffbd62bf97 100644 --- a/lisp/org/ob-scheme.el +++ b/lisp/org/ob-scheme.el @@ -53,8 +53,6 @@ geiser-repl-window-allow-split (declare-function run-geiser "ext:geiser-repl" (impl)) (declare-function geiser-mode "ext:geiser-mode" ()) -(declare-function geiser-eval-region "ext:geiser-mode" - (start end &optional and-go raw nomsg)) (declare-function geiser-repl-exit "ext:geiser-repl" (&optional arg)) (declare-function geiser-eval--retort-output "ext:geiser-eval" (ret)) (declare-function geiser-eval--retort-result-str "ext:geiser-eval" (ret prefix)) @@ -176,7 +174,14 @@ org-babel-scheme-execute-with-geiser (setq geiser-impl--implementation nil) (let ((geiser-debug-jump-to-debug-p nil) (geiser-debug-show-debug-p nil)) - (let ((ret (geiser-eval-region (point-min) (point-max + (let ((ret (funcall + ;; use `geiser-eval-region/wait' only when available + ;; in newer versions of `geiser' + (if (fboundp 'geiser-eval-region/wait) + 'geiser-eval-region/wait + 'geiser-eval-region) + (point-min) + (point-max (setq result (if output (or (geiser-eval--retort-output ret) "Geiser Interpreter produced no output")
Re: [PATCH] update ob-scheme to latest changes in geiser package
On 30/12/2021 08:53, Felipe Lema wrote: On Wednesday, 29 December 2021 15.03.47 -03 Felipe Lema wrote: The geiser package had a recent update in which `geiser-eval-region` behaves like an async function and does not return evaluation result. In exchange, the newly introduced `geiser-eval-region/wait` does that now. - (let ((ret (geiser-eval-region (point-min) (point-max + (let ((ret (geiser-eval-region/wait (point-min) (point-max I am not a geiser user, so maybe I missed something. Does this change mean that org becomes incompatible with older geiser versions? E,g, debian and ubuntu have elpa-geiser system package. I would consider testing if `geiser-eval-region/wait' is bound and fallback to `geiser-eval-region' otherwise.
Re: [PATCH] update ob-scheme to latest changes in geiser package
On Wednesday, 29 December 2021 15.03.47 -03 Felipe Lema wrote: > Sup, y'all > > The geiser package had a recent update in which `geiser-eval-region` behaves > like an async function and does not return evaluation result. In exchange, > the newly introduced `geiser-eval-region/wait` does that now. > > I've attached a patch to update ob-scheme.el to reflect these changes. > Without them, evaluating scheme dialect code blocks in Org will end up with > empty results. > > To learn more about the change (and how it impacted other people's flows) see > issue thread below > > https://gitlab.com/emacs-geiser/geiser/-/issues/30 > > Felipe > > I was pointed out that I was missing the corresponding `declare-function`. I've included it in the attached patch. Sorry about the double-posting Felipe update to new API call to evaluate region and wait for result * ob-scheme.el replace `geiser-eval-region` with `geiser-eval-region/wait` as the former changed expected behaviour; the latter returns the result of evaluating. diff --git a/lisp/org/ob-scheme.el b/lisp/org/ob-scheme.el index f4836b23fe..bcceba1258 100644 --- a/lisp/org/ob-scheme.el +++ b/lisp/org/ob-scheme.el @@ -53,8 +53,8 @@ geiser-repl-window-allow-split (declare-function run-geiser "ext:geiser-repl" (impl)) (declare-function geiser-mode "ext:geiser-mode" ()) -(declare-function geiser-eval-region "ext:geiser-mode" - (start end &optional and-go raw nomsg)) +(declare-function geiser-eval-region/wait "ext:geiser-mode" + (start end &optional timeout)) (declare-function geiser-repl-exit "ext:geiser-repl" (&optional arg)) (declare-function geiser-eval--retort-output "ext:geiser-eval" (ret)) (declare-function geiser-eval--retort-result-str "ext:geiser-eval" (ret prefix)) @@ -176,7 +176,7 @@ org-babel-scheme-execute-with-geiser (setq geiser-impl--implementation nil) (let ((geiser-debug-jump-to-debug-p nil) (geiser-debug-show-debug-p nil)) - (let ((ret (geiser-eval-region (point-min) (point-max + (let ((ret (geiser-eval-region/wait (point-min) (point-max (setq result (if output (or (geiser-eval--retort-output ret) "Geiser Interpreter produced no output")
[PATCH] update ob-scheme to latest changes in geiser package
Sup, y'all The geiser package had a recent update in which `geiser-eval-region` behaves like an async function and does not return evaluation result. In exchange, the newly introduced `geiser-eval-region/wait` does that now. I've attached a patch to update ob-scheme.el to reflect these changes. Without them, evaluating scheme dialect code blocks in Org will end up with empty results. To learn more about the change (and how it impacted other people's flows) see issue thread below https://gitlab.com/emacs-geiser/geiser/-/issues/30 Felipe update to new API call to evaluate region and wait for result * ob-scheme.el replace `geiser-eval-region` with `geiser-eval-region/wait` as the former changed expected behaviour; the latter returns the result of evaluating. diff --git a/lisp/org/ob-scheme.el b/lisp/org/ob-scheme.el index f4836b23fe..89c6abf686 100644 --- a/lisp/org/ob-scheme.el +++ b/lisp/org/ob-scheme.el @@ -176,7 +176,7 @@ org-babel-scheme-execute-with-geiser (setq geiser-impl--implementation nil) (let ((geiser-debug-jump-to-debug-p nil) (geiser-debug-show-debug-p nil)) - (let ((ret (geiser-eval-region (point-min) (point-max + (let ((ret (geiser-eval-region/wait (point-min) (point-max (setq result (if output (or (geiser-eval--retort-output ret) "Geiser Interpreter produced no output")