emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* [PATCH] Emacs Org Babel Scheme (Geiser) support
@ 2013-08-05 23:05 Bruno Félix Rezende Ribeiro
  2013-08-06 23:32 ` Michael Gauland
  0 siblings, 1 reply; 3+ messages in thread
From: Bruno Félix Rezende Ribeiro @ 2013-08-05 23:05 UTC (permalink / raw)
  To: emacs-orgmode


[-- Attachment #1.1: Type: text/plain, Size: 5811 bytes --]

	   _________________________________________________

	    [PATCH] EMACS ORG BABEL SCHEME (GEISER) SUPPORT

		      Bruno Félix Rezende Ribeiro
	   _________________________________________________


This is a patch submission for the Emacs Org Babel Scheme support,
Geiser based, implementation (file ob-scheme.el).  It achieves the
following:

1. Removes the restriction on evaluated code's top-level definitions
   introduced in the major rewrite to support Geiser[1].  The previous
   evaluation procedure used to wrap the whole code in a
   `(with-output-to-string ...)' form before sending it to Geiser.  A
   more robust technique is employed in the evaluation procedure
   avoiding such arguable bugs that results from the artificial wrapping
   of code.  Now the Scheme code is not modified in any way before being
   interpreted by the REPL.  For instance it is possible to use the so
   common and necessary `(define ...)' top level statements[2].
2. A more sound and reliable communication mechanism is employed to
   process the return value, output and error yielded by the REPL and
   reported by Geiser to ob-scheme[3].  Previously ob-scheme and Geiser
   did communication over the echo area.[4] That behavior leads to
   inconsistent results when debugging code and potentially broken code
   if a minor change in Geiser's echo area output syntax were made[5].
3. Taking advantage of the new communication mechanism described in the
   previous item the evaluation error handling logic is extended to
   report REPL error messages directly into the result blocks instead of
   the ubiquitous and uninformative "An error occurred."  message.
4. Frame window configuration restitution measures are implemented to
   prevent that Geiser spawns a persistent[6] window for each new
   invoked REPL session[7][8].

This patch has been tested only with Guile's Scheme implementation.  But
inasmuch ob-scheme does not interface directly to interpreters, but
rather to Geiser, the code should be implementation agnostic[9].

The ChangeLog follows:

,----
| Babel Scheme:
|
|  * lisp/ob-scheme.el (org-babel-scheme-make-session-name): remove wrap
|    around code to be evaluated and get evaluation result directly from
|    `geiser-eval-region' respecting evaluation error messages and
|    restoring frame's window configuration after it.
|    (org-babel-scheme-get-repl): Restore frame's window configuration
|    after asking Geiser to run the REPL.
|
| These changes fix:
|
| - a major regression from the older implementation that prevents code
|   with top-level definitions from being correctly evaluated.
| - the mechanism of communication with Geiser (before did over the echo
|   area).
| - a bug of reference to a nonexistent echo area message that occurred
|   whenever debugging (edebug) `org-babel-scheme-make-session-name'.
| - the report of evaluation errors.
| - the intrusive creation of REPL windows.
`----

Thank you for your contribution to Free Software.  Best regards.

*Ps:*

- The ChangeLog entry is already included in the commit for Org
  maintainers' pleasure.
- Since the changes made sums up to more than 15 lines, so it does not
  classify as "tiny change", and the code changed is part of Org's core,
  I will probably need to sign a FSF copyright assignment.  That will be
  no burden as I pretend to contribute to GNU Emacs whenever possible.



Footnotes
_________

[1] See [{O} {PATCH} Babel support for scheme using geiser]
(https://lists.gnu.org/archive/html/emacs-orgmode/2013-01/msg00134.html).

[2] In fact, that is the main and original motivation behind this
patch.  Believe it or not the new ob-scheme implementation failed on
my first attempt to evaluate a snippet of scheme that happens to be
the result of the resolution of the first question of the famous [SICP
- Structure and Interpretation of Computer Programs]
(https://mitpress.mit.edu/sicp/sicp.html).  Odd is the fact that it is
a deliberated regression from the older implementation (cf. [1]).

[3] Now it uses the return value of a `geiser-eval-region' call which
is a loose association list containing all information needed.

[4] The echo area message was used by ob-scheme to detect an error on
evaluation and to transform valid result or output into a lisp object.

[5] It was impossible to debug the whole
`org-babel-scheme-execute-with-geiser' procedure because it relied on
the Geiser's echo area output that was generated several forms earlier
and, by the echo area's own nature/mechanism, had gone when the
programmer pressed a key to step to the next stop point in the edebug
process.  Whether that is a edebug or ob-scheme bug is arguable, but
certainly to use the echo area to inter-library communication is not a
good programming practice since the echo area is intended just as a
clue feature for human eyes.

[6] By "persistent window" I mean an auxiliary window that is not
removed in the end of the associated procedure's execution.

[7] Thus, for session-less code blocks (the default), it was making a
persistent new window each time one evaluates the respective code.

[8] Using the current implementation of Geiser --- while ensuring a
forward-compatible and clean interface --- it is not feasible to
prevent Geiser's spawns of REPL windows; our best is to prevent its
persistence.

[9] Nevertheless, if you can, please test it with other Scheme
implementation supported by Geiser --- i.e., Racket --- and report
back.


--
 ,= ,-_-. =.  Bruno Félix Rezende Ribeiro (oitofelix) [0x28D618AF]
((_/)o o(\_)) There is no system but GNU;
 `-'(. .)`-'  Linux-libre is just one of its kernels;
     \_/      All software should be free as in freedom;

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-Babel-Scheme.patch --]
[-- Type: text/x-patch, Size: 3343 bytes --]

From 66ab485b27daad2126080439f79a664c157ff62a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bruno=20F=C3=A9lix=20Rezende=20Ribeiro?=
 <oitofelix@riseup.net>
Date: Mon, 5 Aug 2013 19:38:48 -0300
Subject: [PATCH] Babel Scheme:

* lisp/ob-scheme.el (org-babel-scheme-make-session-name): remove wrap
  around code to be evaluated and get evaluation result directly from
  eiser-eval-region' respecting evaluation error messages and
  restoring frame's window configuration after it.
  (org-babel-scheme-get-repl): Restore frame's window configuration
  after asking Geiser to run the REPL.

These changes fix:

- a major regression from the older implementation that prevents code
  with top-level definitions from being correctly evaluated.
- the mechanism of communication with Geiser (before did over the echo
  area).
- a bug of reference to a nonexistent echo area message that occurred
  whenever debugging (edebug) rg-babel-scheme-make-session-name'.
- the report of evaluation errors.
- the intrusive creation of REPL windows.
---
 lisp/ob-scheme.el |   26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/lisp/ob-scheme.el b/lisp/ob-scheme.el
index f979640..d002c93 100644
--- a/lisp/ob-scheme.el
+++ b/lisp/ob-scheme.el
@@ -91,15 +91,18 @@
 
 (defun org-babel-scheme-get-repl (impl name)
   "Switch to a scheme REPL, creating it if it doesn't exist:"
-  (let ((buffer (org-babel-scheme-get-session-buffer name)))
+  (let ((buffer (org-babel-scheme-get-session-buffer name))
+	(window-cfg (current-window-configuration)))
     (or buffer
 	(progn
 	  (run-geiser impl)
+	  (setq buffer (current-buffer))
 	  (if name
 	      (progn
 		(rename-buffer name t)
-		(org-babel-scheme-set-session-buffer name (current-buffer))))
-	  (current-buffer)))))
+		(org-babel-scheme-set-session-buffer name buffer)))
+	  (set-window-configuration window-cfg)
+	  buffer))))
 
 (defun org-babel-scheme-make-session-name (buffer name impl)
   "Generate a name for the session buffer.
@@ -127,9 +130,7 @@ 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))))
@@ -141,11 +142,16 @@ is true; otherwise returns the last value."
 			     (current-buffer)))))
 	(setq geiser-repl--repl repl-buffer)
 	(setq geiser-impl--implementation nil)
-	(geiser-eval-region (point-min) (point-max))
 	(setq result
-	      (if (equal (substring (current-message) 0 3) "=> ")
-		  (replace-regexp-in-string "^=> " "" (current-message))
-		"\"An error occurred.\""))
+	      (let ((window-cfg (current-window-configuration))
+		    (pre-result (geiser-eval-region (point-min)
+						    (point-max))))
+		(set-window-configuration window-cfg)
+		(if (or (assq 'error pre-result)
+			output)
+		    (format "\"%s\""
+			    (cdr (assq 'output pre-result)))
+		  (cadr (assq 'result pre-result)))))
 	(when (not repl)
 	  (save-current-buffer (set-buffer repl-buffer)
 			       (geiser-repl-exit))
-- 
1.7.9.6


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] Emacs Org Babel Scheme (Geiser) support
  2013-08-05 23:05 [PATCH] Emacs Org Babel Scheme (Geiser) support Bruno Félix Rezende Ribeiro
@ 2013-08-06 23:32 ` Michael Gauland
  2013-08-08  2:33   ` Bruno Félix Rezende Ribeiro
  0 siblings, 1 reply; 3+ messages in thread
From: Michael Gauland @ 2013-08-06 23:32 UTC (permalink / raw)
  To: emacs-orgmode

Thanks for such a well-written, well-documented, and most of all useful
contribution!  Definitely a big improvement over my initial implementation.

I've applied the patch to my system, but I'm having trouble getting it to
work--I'm not getting any results. For example, this block:

#+BEGIN_SRC scheme 
(display "This is the output")
"This is the value"
#+END_SRC 

Returns nil, whether I'm asking for output or value. I'm running emacs
23.4.1 on Debian wheezy, with Geiser 3.0.

Could you send me a copy of your ob-scheme.el to help me track this down?

Kind Regards,
Mike

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] Emacs Org Babel Scheme (Geiser) support
  2013-08-06 23:32 ` Michael Gauland
@ 2013-08-08  2:33   ` Bruno Félix Rezende Ribeiro
  0 siblings, 0 replies; 3+ messages in thread
From: Bruno Félix Rezende Ribeiro @ 2013-08-08  2:33 UTC (permalink / raw)
  To: emacs-orgmode


[-- Attachment #1.1: Type: text/plain, Size: 1383 bytes --]

Em Tue, 6 Aug 2013 23:32:20 +0000 (UTC)
Michael Gauland <mikelygee@amuri.net> escreveu:

> Thanks for such a well-written, well-documented, and most of all
> useful contribution!  Definitely a big improvement over my initial
> implementation.

Thank you --- you're welcome.  I'm also very grateful for your
contribution to Babel.

> I've applied the patch to my system, but I'm having trouble getting
> it to work--I'm not getting any results. For example, this block:
>
> #+BEGIN_SRC scheme
> (display "This is the output")
> "This is the value"
> #+END_SRC
>
> Returns nil, whether I'm asking for output or value.

Your example is yielding the intended results on my system.

> I'm running emacs 23.4.1 on Debian wheezy, with Geiser 3.0.

I'm running GNU Emacs 24.1 on my own GNU, with Geiser 0.4.  That is
the latest version of Geiser (May 2013)[fn:1], perhaps your problem
resides there[fn:2].

> Could you send me a copy of your ob-scheme.el to help me track this
> down?

Sure.  It is attached.

--
 ,= ,-_-. =.  Bruno Félix Rezende Ribeiro (oitofelix) [0x28D618AF]
((_/)o o(\_)) There is no system but GNU;
 `-'(. .)`-'  Linux-libre is just one of its kernels;
     \_/      All software should be free as in freedom;

* Footnotes

[fn:1] http://geiser.nongnu.org/

[fn:2] I wonder whether you have misspelled your Geiser version.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: ob-scheme.el --]
[-- Type: text/x-emacs-lisp, Size: 7396 bytes --]

;;; ob-scheme.el --- org-babel functions for Scheme

;; Copyright (C) 2010-2013 Free Software Foundation, Inc.

;; Authors: Eric Schulte, Michael Gauland
;; Keywords: literate programming, reproducible research, scheme
;; Homepage: http://orgmode.org

;; This file is part of GNU Emacs.

;; GNU Emacs is free software: you can redistribute it and/or modify
;; it under the terms of the GNU General Public License as published by
;; the Free Software Foundation, either version 3 of the License, or
;; (at your option) any later version.

;; GNU Emacs is distributed in the hope that it will be useful,
;; but WITHOUT ANY WARRANTY; without even the implied warranty of
;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
;; GNU General Public License for more details.

;; You should have received a copy of the GNU General Public License
;; along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.

;;; Commentary:

;; Now working with SBCL for both session and external evaluation.
;;
;; This certainly isn't optimally robust, but it seems to be working
;; for the basic use cases.

;;; Requirements:

;; - a working scheme implementation
;;   (e.g. guile http://www.gnu.org/software/guile/guile.html)
;;
;; - for session based evaluation geiser is required, which is available from
;;   ELPA.

;;; Code:
(require 'ob)
(require 'geiser nil t)
(defvar geiser-repl--repl)             ; Defined in geiser-repl.el
(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

(declare-function run-geiser "geiser-repl" (impl))
(declare-function geiser-mode "geiser-mode" ())
(declare-function geiser-eval-region "geiser-mode" (start end &optional and-go raw nomsg))
(declare-function geiser-repl-exit "geiser-repl" (&optional arg))

(defvar org-babel-default-header-args:scheme '()
  "Default header arguments for scheme code blocks.")

(defun org-babel-expand-body:scheme (body params)
  "Expand BODY according to PARAMS, return the expanded body."
  (let ((vars (mapcar #'cdr (org-babel-get-header params :var))))
    (if (> (length vars) 0)
        (concat "(let ("
                (mapconcat
                 (lambda (var) (format "%S" (print `(,(car var) ',(cdr var)))))
                 vars "\n      ")
                ")\n" body ")")
      body)))


(defvar org-babel-scheme-repl-map (make-hash-table :test 'equal)
  "Map of scheme sessions to session names.")

(defun org-babel-scheme-cleanse-repl-map ()
  "Remove dead buffers from the REPL map."
  (maphash
   (lambda (x y)
     (when (not (buffer-name y))
       (remhash x org-babel-scheme-repl-map)))
   org-babel-scheme-repl-map))

(defun org-babel-scheme-get-session-buffer (session-name)
  "Look up the scheme buffer for a session; return nil if it doesn't exist."
  (org-babel-scheme-cleanse-repl-map) ; Prune dead sessions
  (gethash session-name org-babel-scheme-repl-map))

(defun org-babel-scheme-set-session-buffer (session-name buffer)
  "Record the scheme buffer used for a given session."
  (puthash session-name buffer org-babel-scheme-repl-map))

(defun org-babel-scheme-get-buffer-impl (buffer)
  "Returns the scheme implementation geiser associates with the buffer."
  (with-current-buffer (set-buffer buffer)
    geiser-impl--implementation))

(defun org-babel-scheme-get-repl (impl name)
  "Switch to a scheme REPL, creating it if it doesn't exist:"
  (let ((buffer (org-babel-scheme-get-session-buffer name))
	(window-cfg (current-window-configuration)))
    (or buffer
	(progn
	  (run-geiser impl)
	  (setq buffer (current-buffer))
	  (if name
	      (progn
		(rename-buffer name t)
		(org-babel-scheme-set-session-buffer name buffer)))
	  (set-window-configuration window-cfg)
	  buffer))))

(defun org-babel-scheme-make-session-name (buffer name impl)
  "Generate a name for the session buffer.

For a named session, the buffer name will be the session name.

If the session is unnamed (nil), generate a name.

If the session is 'none', use nil for the session name, and
org-babel-scheme-execute-with-geiser will use a temporary session."
  (let ((result
	 (cond ((not name)
		(concat buffer " " (symbol-name impl) " REPL"))
	       ((string= name "none") nil)
	       (name))))
    result))

(defun org-babel-scheme-execute-with-geiser (code output impl repl)
  "Execute code in specified REPL. If the REPL doesn't exist, create it
using the given scheme implementation.

Returns the output of executing the code if the output parameter
is true; otherwise returns the last value."
  (let ((result nil))
    (with-temp-buffer
      (insert (format ";; -*- geiser-scheme-implementation: %s -*-" impl))
      (newline)
      (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
	      (let ((window-cfg (current-window-configuration))
		    (pre-result (geiser-eval-region (point-min)
						    (point-max))))
		(set-window-configuration window-cfg)
		(if (or (assq 'error pre-result)
			output)
		    (format "\"%s\""
			    (cdr (assq 'output pre-result)))
		  (cadr (assq 'result pre-result)))))
	(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
		       (read result)))))
    result))

(defun org-babel-execute:scheme (body params)
  "Execute a block of Scheme code with org-babel.
This function is called by `org-babel-execute-src-block'"
  (let* ((source-buffer (current-buffer))
	 (source-buffer-name (replace-regexp-in-string ;; zap surrounding *
			      "^ ?\\*\\([^*]+\\)\\*" "\\1"
			      (buffer-name source-buffer))))
    (save-excursion
      (org-babel-reassemble-table
       (let* ((result-type (cdr (assoc :result-type params)))
	      (impl (or (when (cdr (assoc :scheme params))
			  (intern (cdr (assoc :scheme params))))
			geiser-default-implementation
			(car geiser-active-implementations)))
	      (session (org-babel-scheme-make-session-name
			source-buffer-name (cdr (assoc :session params)) impl))
	      (full-body (org-babel-expand-body:scheme body params)))
	 (org-babel-scheme-execute-with-geiser
	  full-body			 ; code
	  (string= result-type "output") ; output?
	  impl				 ; implementation
	  (and (not (string= session "none")) session))) ; session
       (org-babel-pick-name (cdr (assoc :colname-names params))
			    (cdr (assoc :colnames params)))
       (org-babel-pick-name (cdr (assoc :rowname-names params))
			    (cdr (assoc :rownames params)))))))

(provide 'ob-scheme)

;;; ob-scheme.el ends here

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2013-08-08  2:28 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-05 23:05 [PATCH] Emacs Org Babel Scheme (Geiser) support Bruno Félix Rezende Ribeiro
2013-08-06 23:32 ` Michael Gauland
2013-08-08  2:33   ` Bruno Félix Rezende Ribeiro

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).