Scheme output results

Scheme output results

[Neil Jerram]

I don't think that Scheme output results are correctly handled. Please 
consider:

#+BEGIN_SRC scheme :results output
(display "a") (newline)
(display "b") (newline)
(display "c") (newline)
'(a b c)
#+END_SRC

#+RESULTS:
: "a\nb\nc\n"

As compared with the Elisp equivalent:

#+BEGIN_SRC elisp :results output
(princ "a") (terpri)
(princ "b") (terpri)
(princ "c") (terpri)
'(a b c)
#+END_SRC

#+RESULTS:
: a
: b
: c

I have a possible fix for this - on another computer, so not to hand 
right now - but thought it would be check first whether you agree with 
me that the Scheme results should be more like the Elisp ones.

Regards - Neil

Re: Scheme output results

[Nicolas Goaziou]

Hello,

Neil Jerram <neil@ossau.homelinux.net> writes:

> I don't think that Scheme output results are correctly handled. Please 
> consider:
>
> #+BEGIN_SRC scheme :results output
> (display "a") (newline)
> (display "b") (newline)
> (display "c") (newline)
> '(a b c)
> #+END_SRC
>
>
> #+RESULTS: : "a\nb\nc\n"
>
> As compared with the Elisp equivalent:
>
> #+BEGIN_SRC elisp :results output
> (princ "a") (terpri)
> (princ "b") (terpri)
> (princ "c") (terpri)
> '(a b c)
> #+END_SRC
>
> #+RESULTS:
> : a
> : b
> : c
>
> I have a possible fix for this - on another computer, so not to hand 
> right now - but thought it would be check first whether you agree with 
> me that the Scheme results should be more like the Elisp ones.

That sounds like a good idea. Could you send your fix as a proper patch
with a commit message?

Thank you!

Regards,

-- 
Nicolas Goaziou

Re: Scheme output results

[Neil Jerram]

[-- Attachment #1: Type: text/plain, Size: 1677 bytes --]

On 23/07/17 09:52, Nicolas Goaziou wrote:
> Hello,
>
> Neil Jerram <neil@ossau.homelinux.net> writes:
>
>> I don't think that Scheme output results are correctly handled. Please
>> consider:
>>
>> #+BEGIN_SRC scheme :results output
>> (display "a") (newline)
>> (display "b") (newline)
>> (display "c") (newline)
>> '(a b c)
>> #+END_SRC
>>
>>
>> #+RESULTS: : "a\nb\nc\n"
>>
>> As compared with the Elisp equivalent:
>>
>> #+BEGIN_SRC elisp :results output
>> (princ "a") (terpri)
>> (princ "b") (terpri)
>> (princ "c") (terpri)
>> '(a b c)
>> #+END_SRC
>>
>> #+RESULTS:
>> : a
>> : b
>> : c
>>
>> I have a possible fix for this - on another computer, so not to hand
>> right now - but thought it would be check first whether you agree with
>> me that the Scheme results should be more like the Elisp ones.
> That sounds like a good idea. Could you send your fix as a proper patch
> with a commit message?
>
> Thank you!
>
> Regards,

With apologies for the slow follow up, attached is a patch that works 
well for me.

I think a concern with it would be whether the Geiser retort object and 
functions have always existed, and whether we can rely on them 
continuing to exist.  But - at least for me - every Scheme evaluation 
was previously giving "An error occurred", because of an unexpected 
extra "Mark set" message, so I am pretty sure that my patch is an 
improvement.

I guess (following any revisions) you will also need assignment papers.  
I believe I have these on file for Guile already (since about 1990), and 
it's possible that those are worded so as to cover Emacs as well.  Would 
you mind checking, as I've forgotten how to do that myself?

Regards - Neil


[-- Attachment #2: 0001-Improve-Scheme-code-evaluation.patch --]
[-- Type: text/x-patch, Size: 3559 bytes --]

From a02b60b88bcfd1896c05cd858ae80bb1ef183730 Mon Sep 17 00:00:00 2001
From: Neil Jerram <neil@tigera.io>
Date: Sun, 20 Aug 2017 17:25:58 +0100
Subject: [PATCH] Improve Scheme code evaluation

* lisp/ob-scheme.el (org-babel-scheme-execute-with-geiser): Use
  Geiser's explicit 'retort' object, instead of catching and parsing
  an expected Emacs message (which for me is in any case overwritten
  by a following "Mark set" message); this also means we don't need
  the with-output-to-string approach for getting 'output' results.
  Also 'let' Geiser variables so as to avoid popping up the REPL
  and *Geiser dbg* buffers.
---
 lisp/ob-scheme.el | 56 +++++++++++++++++++++++++++----------------------------
 1 file changed, 28 insertions(+), 28 deletions(-)

diff --git a/lisp/ob-scheme.el b/lisp/ob-scheme.el
index b8867d5f6..bf1da01b0 100644
--- a/lisp/ob-scheme.el
+++ b/lisp/ob-scheme.el
@@ -150,35 +150,35 @@ is true; otherwise returns the last value."
     (with-temp-buffer
       (insert (format ";; -*- geiser-scheme-implementation: %s -*-" impl))
       (newline)
-      (insert (if output
-		  (format "(with-output-to-string (lambda () %s))" code)
-		code))
+      (insert code)
       (geiser-mode)
-      (let ((repl-buffer (save-current-buffer
-			   (org-babel-scheme-get-repl impl repl))))
-	(when (not (eq impl (org-babel-scheme-get-buffer-impl
-			     (current-buffer))))
-	  (message "Implementation mismatch: %s (%s) %s (%s)" impl (symbolp impl)
-		   (org-babel-scheme-get-buffer-impl (current-buffer))
-		   (symbolp (org-babel-scheme-get-buffer-impl
-			     (current-buffer)))))
-	(setq geiser-repl--repl repl-buffer)
-	(setq geiser-impl--implementation nil)
-	(setq result (org-babel-scheme-capture-current-message
-		      (geiser-eval-region (point-min) (point-max))))
-	(setq result
-	      (if (and (stringp result) (equal (substring result 0 3) "=> "))
-		  (replace-regexp-in-string "^=> " "" result)
-		"\"An error occurred.\""))
-	(when (not repl)
-	  (save-current-buffer (set-buffer repl-buffer)
-			       (geiser-repl-exit))
-	  (set-process-query-on-exit-flag (get-buffer-process repl-buffer) nil)
-	  (kill-buffer repl-buffer))
-	(setq result (if (or (string= result "#<void>")
-			     (string= result "#<unspecified>"))
-			 nil
-		       result))))
+      (let ((geiser-repl-window-allow-split nil)
+	    (geiser-repl-use-other-window nil))
+	(let ((repl-buffer (save-current-buffer
+			     (org-babel-scheme-get-repl impl repl))))
+	  (when (not (eq impl (org-babel-scheme-get-buffer-impl
+			       (current-buffer))))
+	    (message "Implementation mismatch: %s (%s) %s (%s)" impl (symbolp impl)
+		     (org-babel-scheme-get-buffer-impl (current-buffer))
+		     (symbolp (org-babel-scheme-get-buffer-impl
+			       (current-buffer)))))
+	  (setq geiser-repl--repl repl-buffer)
+	  (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))))
+	      (setq result (if output
+			       (geiser-eval--retort-output ret)
+			     (geiser-eval--retort-result-str ret "")))))
+	  (when (not repl)
+	    (save-current-buffer (set-buffer repl-buffer)
+				 (geiser-repl-exit))
+	    (set-process-query-on-exit-flag (get-buffer-process repl-buffer) nil)
+	    (kill-buffer repl-buffer))
+	  (setq result (if (or (string= result "#<void>")
+			       (string= result "#<unspecified>"))
+			   nil
+			 result)))))
     result))
 
 (defun org-babel-scheme--table-or-string (results)
-- 
2.13.2

Re: Scheme output results

[Nicolas Goaziou]

Hello,

Neil Jerram <neil@ossau.homelinux.net> writes:

> With apologies for the slow follow up, attached is a patch that works
> well for me.

Thank you.

> I guess (following any revisions) you will also need assignment
> papers.  I believe I have these on file for Guile already (since about
> 1990), and it's possible that those are worded so as to cover Emacs as
> well.  Would you mind checking, as I've forgotten how to do that
> myself?

I cannot, I'm not the maintainer. I'm Cc'ing Bastien. I'll apply your
patch as soon as it is sorted out.

Regards,

-- 
Nicolas Goaziou

Re: Scheme output results

[Bastien Guerry]

Hi Neil,

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> I cannot, I'm not the maintainer. I'm Cc'ing Bastien. I'll apply your
> patch as soon as it is sorted out.

Thanks for contributing.

I’ve found your copyright assignment for GUILE but it does not cover
changes for GNU Emacs, you would need another copyright assignment for
this.

Thanks in advance,

-- 
 Bastien

Re: Scheme output results

[Neil Jerram]

Hi Bastien,

Thanks for checking this. I've written to assign@gnu.org to kick off sorting out the additional assignment.

Regards - Neil


  Original Message  
From: Bastien Guerry
Sent: Sunday, 27 August 2017 13:07
To: Nicolas Goaziou
Cc: Neil Jerram; emacs-orgmode@gnu.org
Subject: Re: [O] Scheme output results

Hi Neil,

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> I cannot, I'm not the maintainer. I'm Cc'ing Bastien. I'll apply your
> patch as soon as it is sorted out.

Thanks for contributing.

I’ve found your copyright assignment for GUILE but it does not cover
changes for GNU Emacs, you would need another copyright assignment for
this.

Thanks in advance,

-- 
Bastien

Re: Scheme output results

[Neil Jerram]

Hi Bastien & Nicolas,

I've been told by "Theodore Teah via RT <copyright-clerk@fsf.org>" that 
my copyright assignment is in place now.

Regards - Neil



On 31/08/17 13:21, Neil Jerram wrote:
> Hi Bastien,
>
> Thanks for checking this. I've written to assign@gnu.org to kick off sorting out the additional assignment.
>
> Regards - Neil
>
>
>    Original Message
> From: Bastien Guerry
> Sent: Sunday, 27 August 2017 13:07
> To: Nicolas Goaziou
> Cc: Neil Jerram; emacs-orgmode@gnu.org
> Subject: Re: [O] Scheme output results
>
> Hi Neil,
>
> Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:
>
>> I cannot, I'm not the maintainer. I'm Cc'ing Bastien. I'll apply your
>> patch as soon as it is sorted out.
> Thanks for contributing.
>
> I’ve found your copyright assignment for GUILE but it does not cover
> changes for GNU Emacs, you would need another copyright assignment for
> this.
>
> Thanks in advance,
>

Re: Scheme output results

[Nicolas Goaziou]

Hello,

Neil Jerram <neil@ossau.homelinux.net> writes:

> Hi Bastien & Nicolas,
>
> I've been told by "Theodore Teah via RT <copyright-clerk@fsf.org>"
> that my copyright assignment is in place now.

Great!

I tried to build Org with your patch, but it generates compilation
warnings. Could you look into it?

Thank you.

Regards,

-- 
Nicolas Goaziou

Re: Scheme output results

[Neil Jerram]

[-- Attachment #1: Type: text/plain, Size: 479 bytes --]

Hi Nicolas,

On 05/09/17 22:49, Nicolas Goaziou wrote:
> Hello,
>
> Neil Jerram <neil@ossau.homelinux.net> writes:
>
>> Hi Bastien & Nicolas,
>>
>> I've been told by "Theodore Teah via RT <copyright-clerk@fsf.org>"
>> that my copyright assignment is in place now.
> Great!
>
> I tried to build Org with your patch, but it generates compilation
> warnings. Could you look into it?
>
> Thank you.
>
> Regards,
>

I believe the attached patch fixes those problems.

Regards - Neil


[-- Attachment #2: 0001-Improve-Scheme-code-evaluation.patch --]
[-- Type: text/x-patch, Size: 4630 bytes --]

From 35d3742774335decfcf8e4e5ab8e4101332a39a5 Mon Sep 17 00:00:00 2001
From: Neil Jerram <neil@tigera.io>
Date: Sun, 20 Aug 2017 17:25:58 +0100
Subject: [PATCH] Improve Scheme code evaluation

* lisp/ob-scheme.el (org-babel-scheme-execute-with-geiser): Use
  Geiser's explicit 'retort' object, instead of catching and parsing
  an expected Emacs message (which for me is in any case overwritten
  by a following "Mark set" message); this also means we don't need
  the with-output-to-string approach for getting 'output' results.
  Also 'let' Geiser variables so as to avoid popping up the REPL
  and *Geiser dbg* buffers.
---
 lisp/ob-scheme.el | 62 ++++++++++++++++++++++++++++++-------------------------
 1 file changed, 34 insertions(+), 28 deletions(-)

diff --git a/lisp/ob-scheme.el b/lisp/ob-scheme.el
index b8867d5..f822e15 100644
--- a/lisp/ob-scheme.el
+++ b/lisp/ob-scheme.el
@@ -44,12 +44,18 @@
 (defvar geiser-impl--implementation)   ; Defined in geiser-impl.el
 (defvar geiser-default-implementation) ; Defined in geiser-impl.el
 (defvar geiser-active-implementations) ; Defined in geiser-impl.el
+(defvar geiser-debug-show-debug-p)     ; Defined in geiser-debug.el
+(defvar geiser-debug-jump-to-debug-p)  ; Defined in geiser-debug.el
+(defvar geiser-repl-use-other-window)  ; Defined in geiser-repl.el
+(defvar geiser-repl-window-allow-split)	; Defined in geiser-repl.el
 
 (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))
 
 (defcustom org-babel-scheme-null-to 'hline
   "Replace `null' and empty lists in scheme tables with this before returning."
@@ -150,35 +156,35 @@ is true; otherwise returns the last value."
     (with-temp-buffer
       (insert (format ";; -*- geiser-scheme-implementation: %s -*-" impl))
       (newline)
-      (insert (if output
-		  (format "(with-output-to-string (lambda () %s))" code)
-		code))
+      (insert code)
       (geiser-mode)
-      (let ((repl-buffer (save-current-buffer
-			   (org-babel-scheme-get-repl impl repl))))
-	(when (not (eq impl (org-babel-scheme-get-buffer-impl
-			     (current-buffer))))
-	  (message "Implementation mismatch: %s (%s) %s (%s)" impl (symbolp impl)
-		   (org-babel-scheme-get-buffer-impl (current-buffer))
-		   (symbolp (org-babel-scheme-get-buffer-impl
-			     (current-buffer)))))
-	(setq geiser-repl--repl repl-buffer)
-	(setq geiser-impl--implementation nil)
-	(setq result (org-babel-scheme-capture-current-message
-		      (geiser-eval-region (point-min) (point-max))))
-	(setq result
-	      (if (and (stringp result) (equal (substring result 0 3) "=> "))
-		  (replace-regexp-in-string "^=> " "" result)
-		"\"An error occurred.\""))
-	(when (not repl)
-	  (save-current-buffer (set-buffer repl-buffer)
-			       (geiser-repl-exit))
-	  (set-process-query-on-exit-flag (get-buffer-process repl-buffer) nil)
-	  (kill-buffer repl-buffer))
-	(setq result (if (or (string= result "#<void>")
-			     (string= result "#<unspecified>"))
-			 nil
-		       result))))
+      (let ((geiser-repl-window-allow-split nil)
+	    (geiser-repl-use-other-window nil))
+	(let ((repl-buffer (save-current-buffer
+			     (org-babel-scheme-get-repl impl repl))))
+	  (when (not (eq impl (org-babel-scheme-get-buffer-impl
+			       (current-buffer))))
+	    (message "Implementation mismatch: %s (%s) %s (%s)" impl (symbolp impl)
+		     (org-babel-scheme-get-buffer-impl (current-buffer))
+		     (symbolp (org-babel-scheme-get-buffer-impl
+			       (current-buffer)))))
+	  (setq geiser-repl--repl repl-buffer)
+	  (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))))
+	      (setq result (if output
+			       (geiser-eval--retort-output ret)
+			     (geiser-eval--retort-result-str ret "")))))
+	  (when (not repl)
+	    (save-current-buffer (set-buffer repl-buffer)
+				 (geiser-repl-exit))
+	    (set-process-query-on-exit-flag (get-buffer-process repl-buffer) nil)
+	    (kill-buffer repl-buffer))
+	  (setq result (if (or (string= result "#<void>")
+			       (string= result "#<unspecified>"))
+			   nil
+			 result)))))
     result))
 
 (defun org-babel-scheme--table-or-string (results)
-- 
2.7.4

Re: Scheme output results

[Nicolas Goaziou]

Hello,

Neil Jerram <neil@ossau.homelinux.net> writes:

> I believe the attached patch fixes those problems.

Applied. Thank you!

Regards,

-- 
Nicolas Goaziou