From 6aef0678f12c31e43d1584cf0b32bfcffc7156c0 Mon Sep 17 00:00:00 2001 From: Takafumi Arakaki Date: Sat, 16 Jun 2012 18:45:01 +0200 Subject: [PATCH 01/14] Refactor ein:notebook-execute-current-cell --- ein-cell.el | 15 ++++++----- tests/test-ein-notebook.el | 53 +++++++++++++++++++++----------------- 2 files changed, 37 insertions(+), 31 deletions(-) diff --git a/ein-cell.el b/ein-cell.el index 78f14a2..7de64d5 100644 --- a/ein-cell.el +++ b/ein-cell.el @@ -780,13 +780,14 @@ Called from ewoc pretty printer via `ein:cell-insert-output'." (ein:cell-set-input-prompt cell "*") (ein:cell-running-set cell t) (oset cell :dynamic t) - (let* ((callbacks - (list - :execute_reply (cons #'ein:cell--handle-execute-reply cell) - :output (cons #'ein:cell--handle-output cell) - :clear_output (cons #'ein:cell--handle-clear-output cell) - :set_next_input (cons #'ein:cell--handle-set-next-input cell)))) - (apply #'ein:kernel-execute kernel code callbacks args))) + (apply #'ein:kernel-execute kernel code (ein:cell-make-callbacks cell) args)) + +(defmethod ein:cell-make-callbacks ((cell ein:codecell)) + (list + :execute_reply (cons #'ein:cell--handle-execute-reply cell) + :output (cons #'ein:cell--handle-output cell) + :clear_output (cons #'ein:cell--handle-clear-output cell) + :set_next_input (cons #'ein:cell--handle-set-next-input cell))) (defmethod ein:cell--handle-execute-reply ((cell ein:codecell) content) (ein:cell-set-input-prompt cell (plist-get content :execution_count)) diff --git a/tests/test-ein-notebook.el b/tests/test-ein-notebook.el index 882af6c..4a3f029 100644 --- a/tests/test-ein-notebook.el +++ b/tests/test-ein-notebook.el @@ -70,6 +70,27 @@ (defun eintest:notebook-enable-mode (buffer) (with-current-buffer buffer (ein:notebook-plain-mode) buffer)) +(defun eintest:kernel-fake-execute-reply (kernel msg-id execution-count) + (let* ((payload nil) + (content (list :execution_count 1 :payload payload)) + (packet (list :header (list :msg_type "execute_reply") + :parent_header (list :msg_id msg-id) + :content content))) + (ein:kernel--handle-shell-reply kernel (json-encode packet)))) + +(defun eintest:kernel-fake-stream (kernel msg-id data) + (let* ((content (list :data data + :name "stdout")) + (packet (list :header (list :msg_type "stream") + :parent_header (list :msg_id msg-id) + :content content))) + (ein:kernel--handle-iopub-reply kernel (json-encode packet)))) + +(defun eintest:search-forward-from (string start) + (save-excursion + (goto-char start) + (search-forward string nil t))) + ;; from-json @@ -332,15 +353,11 @@ some text (cell (ein:notebook-get-current-cell)) (kernel (ein:$notebook-kernel ein:notebook)) (msg-id "DUMMY-MSG-ID") - (callbacks - (list - :execute_reply (cons #'ein:cell--handle-execute-reply cell) - :output (cons #'ein:cell--handle-output cell) - :clear_output (cons #'ein:cell--handle-clear-output cell) - :set_next_input (cons #'ein:cell--handle-set-next-input cell)))) + (callbacks (ein:cell-make-callbacks cell))) (should (ein:$kernel-p kernel)) (should (ein:codecell-p cell)) (should (ein:$kernel-p (oref cell :kernel))) + ;; Execute (insert text) (mocker-let ((ein:kernel-execute (kernel code callbacks kwd-silent silent) @@ -350,25 +367,13 @@ some text ((:input (list kernel) :output t)))) (ein:notebook-execute-current-cell)) (ein:kernel-set-callbacks-for-msg kernel msg-id callbacks) - (save-excursion - (goto-char (point-min)) - (should-not (search-forward "In [1]:" nil t))) - (let* ((payload nil) - (content (list :execution_count 1 :payload payload)) - (packet (list :header (list :msg_type "execute_reply") - :parent_header (list :msg_id msg-id) - :content content))) - (ein:kernel--handle-shell-reply kernel (json-encode packet))) + ;; Execute reply + (should-not (eintest:search-forward-from "In [1]:" (point-min))) + (eintest:kernel-fake-execute-reply kernel msg-id 1) (should (= (oref cell :input-prompt-number) 1)) - (save-excursion - (goto-char (point-min)) - (should (search-forward "In [1]:" nil t))) - (let* ((content (list :data "'Hello World'" - :name "stdout")) - (packet (list :header (list :msg_type "stream") - :parent_header (list :msg_id msg-id) - :content content))) - (ein:kernel--handle-iopub-reply kernel (json-encode packet))) + (should (eintest:search-forward-from "In [1]:" (point-min))) + ;; Stream output + (eintest:kernel-fake-stream kernel msg-id "'Hello World'") (should (= (ein:cell-num-outputs cell) 1)) (save-excursion (goto-char (point-min)) From 74a433dd7b108a778aad9e6817fbcf4341f9c0f3 Mon Sep 17 00:00:00 2001 From: Takafumi Arakaki Date: Sat, 16 Jun 2012 18:51:56 +0200 Subject: [PATCH 02/14] Refactor ein:notebook-execute-current-cell more --- tests/test-ein-notebook.el | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/tests/test-ein-notebook.el b/tests/test-ein-notebook.el index 4a3f029..088ae6b 100644 --- a/tests/test-ein-notebook.el +++ b/tests/test-ein-notebook.el @@ -346,6 +346,16 @@ some text ;; Kernel related things +(defun eintest:notebook-fake-execution (kernel text msg-id callbacks) + (mocker-let ((ein:kernel-execute + (kernel code callbacks kwd-silent silent) + ((:input (list kernel text callbacks :silent nil)))) + (ein:kernel-ready-p + (kernel) + ((:input (list kernel) :output t)))) + (ein:notebook-execute-current-cell)) + (ein:kernel-set-callbacks-for-msg kernel msg-id callbacks)) + (ert-deftest ein:notebook-execute-current-cell () (with-current-buffer (eintest:notebook-make-empty) (ein:notebook-insert-cell-below-command) @@ -359,14 +369,7 @@ some text (should (ein:$kernel-p (oref cell :kernel))) ;; Execute (insert text) - (mocker-let ((ein:kernel-execute - (kernel code callbacks kwd-silent silent) - ((:input (list kernel text callbacks :silent nil)))) - (ein:kernel-ready-p - (kernel) - ((:input (list kernel) :output t)))) - (ein:notebook-execute-current-cell)) - (ein:kernel-set-callbacks-for-msg kernel msg-id callbacks) + (eintest:notebook-fake-execution kernel text msg-id callbacks) ;; Execute reply (should-not (eintest:search-forward-from "In [1]:" (point-min))) (eintest:kernel-fake-execute-reply kernel msg-id 1) From c0839fa79515823a1bbef76f3a4b035d3973a497 Mon Sep 17 00:00:00 2001 From: Takafumi Arakaki Date: Sat, 16 Jun 2012 18:58:27 +0200 Subject: [PATCH 03/14] Refactor ein:notebook-execute-current-cell more (2) --- tests/test-ein-notebook.el | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/test-ein-notebook.el b/tests/test-ein-notebook.el index 088ae6b..4bdd446 100644 --- a/tests/test-ein-notebook.el +++ b/tests/test-ein-notebook.el @@ -346,6 +346,11 @@ some text ;; Kernel related things +(defun eintest:notebook-check-kernel-and-codecell (kernel cell) + (should (ein:$kernel-p kernel)) + (should (ein:codecell-p cell)) + (should (ein:$kernel-p (oref cell :kernel)))) + (defun eintest:notebook-fake-execution (kernel text msg-id callbacks) (mocker-let ((ein:kernel-execute (kernel code callbacks kwd-silent silent) @@ -364,9 +369,7 @@ some text (kernel (ein:$notebook-kernel ein:notebook)) (msg-id "DUMMY-MSG-ID") (callbacks (ein:cell-make-callbacks cell))) - (should (ein:$kernel-p kernel)) - (should (ein:codecell-p cell)) - (should (ein:$kernel-p (oref cell :kernel))) + (eintest:notebook-check-kernel-and-codecell kernel cell) ;; Execute (insert text) (eintest:notebook-fake-execution kernel text msg-id callbacks) From 35c3e9bd7b26fa9a8cf0e95f964c4ef55988a660 Mon Sep 17 00:00:00 2001 From: Takafumi Arakaki Date: Sat, 16 Jun 2012 19:00:44 +0200 Subject: [PATCH 04/14] Fix ein:notebook-execute-current-cell "Hellow World" for eintest:kernel-fake-stream had unnecessary single quotes. --- tests/test-ein-notebook.el | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test-ein-notebook.el b/tests/test-ein-notebook.el index 4bdd446..05fde5d 100644 --- a/tests/test-ein-notebook.el +++ b/tests/test-ein-notebook.el @@ -379,13 +379,13 @@ some text (should (= (oref cell :input-prompt-number) 1)) (should (eintest:search-forward-from "In [1]:" (point-min))) ;; Stream output - (eintest:kernel-fake-stream kernel msg-id "'Hello World'") + (eintest:kernel-fake-stream kernel msg-id "Hello World") (should (= (ein:cell-num-outputs cell) 1)) (save-excursion (goto-char (point-min)) (should (search-forward "In [1]:" nil t)) (should (search-forward "print 'Hello World'" nil t)) - (should (search-forward "Hello World" nil t)) ; stream output + (should (search-forward "\nHello World\n" nil t)) ; stream output (should-not (search-forward "Hello World" nil t)))))) From 1127de66a710f722d385089041d26cc23195f50c Mon Sep 17 00:00:00 2001 From: Takafumi Arakaki Date: Sat, 16 Jun 2012 20:11:51 +0200 Subject: [PATCH 05/14] Add two tests for undo in notebook --- tests/test-ein-notebook.el | 77 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/tests/test-ein-notebook.el b/tests/test-ein-notebook.el index 05fde5d..5f3da2e 100644 --- a/tests/test-ein-notebook.el +++ b/tests/test-ein-notebook.el @@ -388,6 +388,83 @@ some text (should (search-forward "\nHello World\n" nil t)) ; stream output (should-not (search-forward "Hello World" nil t)))))) + +;; Notebook undo + +(ert-deftest ein:notebook-undo-after-execution () + (with-current-buffer (eintest:notebook-make-empty) + (ein:notebook-insert-cell-below-command) + (let* ((text "print 'Hello World'") + (output-text "Hello World\n") + (cell (ein:notebook-get-current-cell)) + (kernel (ein:$notebook-kernel ein:notebook)) + (msg-id "DUMMY-MSG-ID") + (callbacks (ein:cell-make-callbacks cell)) + (check-output + (lambda () + (save-excursion + (goto-char (point-min)) + (should (search-forward (concat "\n" output-text) nil t)) + (should-not (search-forward "Hello World" nil t)))))) + (eintest:notebook-check-kernel-and-codecell kernel cell) + ;; Execute + (insert text) + (undo-boundary) + (eintest:notebook-fake-execution kernel text msg-id callbacks) + (ein:kernel-set-callbacks-for-msg kernel msg-id callbacks) + ;; Stream output + (eintest:kernel-fake-stream kernel msg-id output-text) + (funcall check-output) + ;; Undo + (should (equal (ein:cell-get-text cell) text)) + (undo) + (should (equal (ein:cell-get-text cell) "")) + ;; FIXME: Known bug. (it must succeed.) + (should-error (funcall check-output))))) + +(ert-deftest ein:notebook-undo-after-execution-2-cells () + (with-current-buffer (eintest:notebook-make-empty) + (ein:notebook-insert-cell-below-command) + (ein:notebook-insert-cell-above-command) + (let* ((text "print 'Hello World\\n' * 10") + (next-text "something") + (output-text + (apply #'concat (loop repeat 10 collect "Hello World\n"))) + (cell (ein:notebook-get-current-cell)) + (next-cell (ein:cell-next cell)) + (kernel (ein:$notebook-kernel ein:notebook)) + (msg-id "DUMMY-MSG-ID") + (callbacks (ein:cell-make-callbacks cell)) + (check-output + (lambda () + (save-excursion + (goto-char (point-min)) + (should (search-forward (concat "\n" output-text) nil t)) + (should-not (search-forward "Hello World" nil t)))))) + (eintest:notebook-check-kernel-and-codecell kernel cell) + ;; Execute + (insert text) + (undo-boundary) + (let ((pos (point))) + ;; Do not use `save-excursion' because it does not record undo. + (ein:notebook-goto-next-input-command) + (insert next-text) + (undo-boundary) + (goto-char pos)) + (eintest:notebook-fake-execution kernel text msg-id callbacks) + (ein:kernel-set-callbacks-for-msg kernel msg-id callbacks) + ;; Stream output + (eintest:kernel-fake-stream kernel msg-id output-text) + (funcall check-output) + ;; Undo + (should (equal (ein:cell-get-text cell) text)) + (should (equal (ein:cell-get-text next-cell) next-text)) + (undo) + (should (equal (ein:cell-get-text cell) text)) + ;; FIXME: Known bug. (these two must succeed.) + (should-error (should (equal (ein:cell-get-text next-cell) ""))) + (should-error (funcall check-output))))) + ;; Notebook mode From 4f3695c4cf184f13e3ddb7c360072b05a4789995 Mon Sep 17 00:00:00 2001 From: Takafumi Arakaki Date: Sat, 16 Jun 2012 20:41:27 +0200 Subject: [PATCH 06/14] Refactor ein:notebook-undo-after-execution* .. and ein:notebook-execute-current-cell. --- tests/test-ein-notebook.el | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/tests/test-ein-notebook.el b/tests/test-ein-notebook.el index 5f3da2e..5cbada4 100644 --- a/tests/test-ein-notebook.el +++ b/tests/test-ein-notebook.el @@ -86,10 +86,16 @@ :content content))) (ein:kernel--handle-iopub-reply kernel (json-encode packet)))) -(defun eintest:search-forward-from (string start) +(defun eintest:check-search-forward-from (start string &optional null-string) + "Search STRING from START and check it is found. +When non-`nil' NULL-STRING is given, it is searched from the +position where the search of the STRING ends and check that it +is not found." (save-excursion (goto-char start) - (search-forward string nil t))) + (should (search-forward string nil t)) + (when null-string + (should-not (search-forward null-string nil t))))) ;; from-json @@ -374,10 +380,10 @@ some text (insert text) (eintest:notebook-fake-execution kernel text msg-id callbacks) ;; Execute reply - (should-not (eintest:search-forward-from "In [1]:" (point-min))) + (should-error (eintest:check-search-forward-from (point-min) "In [1]:")) (eintest:kernel-fake-execute-reply kernel msg-id 1) (should (= (oref cell :input-prompt-number) 1)) - (should (eintest:search-forward-from "In [1]:" (point-min))) + (eintest:check-search-forward-from (point-min) "In [1]:") ;; Stream output (eintest:kernel-fake-stream kernel msg-id "Hello World") (should (= (ein:cell-num-outputs cell) 1)) @@ -402,10 +408,8 @@ some text (callbacks (ein:cell-make-callbacks cell)) (check-output (lambda () - (save-excursion - (goto-char (point-min)) - (should (search-forward (concat "\n" output-text) nil t)) - (should-not (search-forward "Hello World" nil t)))))) + (eintest:check-search-forward-from + (point-min) (concat "\n" output-text) "Hello World")))) (eintest:notebook-check-kernel-and-codecell kernel cell) ;; Execute (insert text) @@ -437,10 +441,8 @@ some text (callbacks (ein:cell-make-callbacks cell)) (check-output (lambda () - (save-excursion - (goto-char (point-min)) - (should (search-forward (concat "\n" output-text) nil t)) - (should-not (search-forward "Hello World" nil t)))))) + (eintest:check-search-forward-from + (point-min) (concat "\n" output-text) "Hello World")))) (eintest:notebook-check-kernel-and-codecell kernel cell) ;; Execute (insert text) From 7bf8acd484c65e6b6def1cdcf2fdc9a0e937c0e1 Mon Sep 17 00:00:00 2001 From: Takafumi Arakaki Date: Sat, 16 Jun 2012 20:42:14 +0200 Subject: [PATCH 07/14] Rename to ein:notebook-undo-after-execution-1-cell --- tests/test-ein-notebook.el | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test-ein-notebook.el b/tests/test-ein-notebook.el index 5cbada4..43658ce 100644 --- a/tests/test-ein-notebook.el +++ b/tests/test-ein-notebook.el @@ -397,7 +397,7 @@ some text ;; Notebook undo -(ert-deftest ein:notebook-undo-after-execution () +(ert-deftest ein:notebook-undo-after-execution-1-cell () (with-current-buffer (eintest:notebook-make-empty) (ein:notebook-insert-cell-below-command) (let* ((text "print 'Hello World'") From 5b06f8fcd8f37692ec8fd9b090ae7ebbebb5b5d6 Mon Sep 17 00:00:00 2001 From: Takafumi Arakaki Date: Sat, 16 Jun 2012 21:04:47 +0200 Subject: [PATCH 08/14] More precise check in undo tests --- ein-cell.el | 4 +++- tests/test-ein-notebook.el | 11 +++++++---- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/ein-cell.el b/ein-cell.el index 7de64d5..7d006a3 100644 --- a/ein-cell.el +++ b/ein-cell.el @@ -543,7 +543,9 @@ Called from ewoc pretty printer via `ein:cell-pp'." (defmethod ein:cell-location ((cell ein:basecell) &optional elm end) "Return the starting location of CELL. -ELM is a name (keyword) of element in the `:element-names' slot of CELL. +ELM is a name (keyword) of element that `ein:cell-element-get' +understands. Note that you can't use `:output' since it returns +a list. Use `:after-input' instead. If END is non-`nil', return the location of next element." (unless elm (setq elm :prompt)) (let ((element (oref cell :element))) diff --git a/tests/test-ein-notebook.el b/tests/test-ein-notebook.el index 43658ce..9f02737 100644 --- a/tests/test-ein-notebook.el +++ b/tests/test-ein-notebook.el @@ -97,6 +97,11 @@ is not found." (when null-string (should-not (search-forward null-string nil t))))) +(defun eintest:cell-check-output (cell regexp) + (save-excursion + (goto-char (ein:cell-location cell :after-input)) + (should (looking-at-p (concat "\\=" regexp "\n"))))) + ;; from-json @@ -408,8 +413,7 @@ some text (callbacks (ein:cell-make-callbacks cell)) (check-output (lambda () - (eintest:check-search-forward-from - (point-min) (concat "\n" output-text) "Hello World")))) + (eintest:cell-check-output cell output-text)))) (eintest:notebook-check-kernel-and-codecell kernel cell) ;; Execute (insert text) @@ -441,8 +445,7 @@ some text (callbacks (ein:cell-make-callbacks cell)) (check-output (lambda () - (eintest:check-search-forward-from - (point-min) (concat "\n" output-text) "Hello World")))) + (eintest:cell-check-output cell output-text)))) (eintest:notebook-check-kernel-and-codecell kernel cell) ;; Execute (insert text) From 311151fdc83984c1c02099908f497a34ee430253 Mon Sep 17 00:00:00 2001 From: Takafumi Arakaki Date: Sat, 16 Jun 2012 21:05:22 +0200 Subject: [PATCH 09/14] Inhibit undo in pretty printer --- ein-notebook.el | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/ein-notebook.el b/ein-notebook.el index e1b46a8..1192abb 100644 --- a/ein-notebook.el +++ b/ein-notebook.el @@ -306,7 +306,14 @@ See `ein:notebook-open' for more information." (defun ein:notebook-pp (ewoc-data) (let ((path (ein:$node-path ewoc-data)) - (data (ein:$node-data ewoc-data))) + (data (ein:$node-data ewoc-data)) + (inhibit-read-only t) + (buffer-undo-list t)) ; disable undo recording + ;; FIXME: Having `inhibit-read-only' and `buffer-undo-list' should + ;; make some of the same statements in ein-notebook/cell.el + ;; removable. These must be removed after careful review + ;; and maybe adding more tests. I will just let them be + ;; there since it is harmless. (case (car path) (cell (ein:cell-pp (cdr path) data))))) From 363b63de04d414cfcd3f3a520b4b1735a66d6443 Mon Sep 17 00:00:00 2001 From: Takafumi Arakaki Date: Sat, 16 Jun 2012 21:35:16 +0200 Subject: [PATCH 10/14] Make undo in notebook configurable Note that ein:notebook-empty-undo-maybe is called in: * ein:notebook-pp * ein:notebook-delete-cell --- ein-notebook.el | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/ein-notebook.el b/ein-notebook.el index 1192abb..cae5b6d 100644 --- a/ein-notebook.el +++ b/ein-notebook.el @@ -54,6 +54,34 @@ ;;; Configuration +(defcustom ein:notebook-enable-undo 'yes + "Configure undo in notebook buffers. + +`no' : symbol + Do not use undo in notebook buffers. It is the safest option. +`yes' : symbol + Enable undo in notebook buffers. You can't undo after + modification of cell (execution, add, remove, etc.). This + is default. +`full' : symbol + Enable full undo in notebook buffers. It is powerful but + sometime (typically after the cell specific commands) undo + mess up notebook buffer. Use it on your own risk. When the + buffer is messed up, you can just redo and continue editing, + or save it once and reopen it if you want to be careful. + +You need to reopen the notebook buffer to reflect the change of +this value." + :type '(choice (const :tag "No" no) + (const :tag "Yes" yes) + (const :tag "Full" full)) + :group 'ein) + +(defun ein:notebook-empty-undo-maybe () + "Empty `buffer-undo-list' if `ein:notebook-enable-undo' is `yes'." + (when (eq ein:notebook-enable-undo 'yes) + (setq buffer-undo-list nil))) + (defcustom ein:notebook-discard-output-on-save 'no "Configure if the output part of the cell should be saved or not. @@ -295,6 +323,8 @@ See `ein:notebook-open' for more information." (assert ein:notebook) ; make sure in a notebook buffer (ein:notebook-from-json ein:notebook (ein:$notebook-data ein:notebook)) (setq buffer-undo-list nil) ; clear undo history + (when (eq ein:notebook-enable-undo 'no) + (setq buffer-undo-list t)) (ein:notebook-mode) (setf (ein:$notebook-notification ein:notebook) (ein:notification-setup (current-buffer))) @@ -315,7 +345,8 @@ See `ein:notebook-open' for more information." ;; and maybe adding more tests. I will just let them be ;; there since it is harmless. (case (car path) - (cell (ein:cell-pp (cdr path) data))))) + (cell (ein:cell-pp (cdr path) data)))) + (ein:notebook-empty-undo-maybe)) ;;; Initialization. @@ -392,7 +423,8 @@ See `ein:notebook-open' for more information." (apply #'ewoc-delete (ein:$notebook-ewoc notebook) (ein:cell-all-element cell))) - (setf (ein:$notebook-dirty notebook) t)) + (setf (ein:$notebook-dirty notebook) t) + (ein:notebook-empty-undo-maybe)) (defun ein:notebook-delete-cell-command () "Delete a cell. \(WARNING: no undo!) From 0c13bc4234bc6c77b10ea379f97f5f31eea8d60b Mon Sep 17 00:00:00 2001 From: Takafumi Arakaki Date: Sat, 16 Jun 2012 22:14:20 +0200 Subject: [PATCH 11/14] Modifying undo in PP is wrong Previous change was not complete since the test does NOT fail. The test must fail because the test was for `full' undo but actually it was tested against normal undo. It turned out let binding of buffer-undo-list in ein-cell.el was the right way to do it, since EWOC does some buffer modification outside of the pretty printer. Therefore, setting inhibit-read-only and buffer-undo-list in the pretty printer makes no sense. Thus, they are removed from ein:notebook-pp. This means that wrapping EWOC functions with let binding of buffer-undo-list is essential. Meaning that what I wrote in the FIXME ein:notebook-pp was wrong. Therefore, ein:notebook-empty-undo-maybe cannot be called in ein:notebook-pp and ein:notebook-empty-undo-maybe must be called in every single functions that modifies buffer via EWOC. Tests for three cases of undo configuration are added. These test still fail: ein:notebook-undo-after-execution-1-cell/yes ein:notebook-undo-after-execution-2-cells/yes --- ein-notebook.el | 26 +++++++++---------- tests/test-ein-notebook.el | 52 ++++++++++++++++++++++++++++++-------- 2 files changed, 54 insertions(+), 24 deletions(-) diff --git a/ein-notebook.el b/ein-notebook.el index cae5b6d..4f3823d 100644 --- a/ein-notebook.el +++ b/ein-notebook.el @@ -336,17 +336,9 @@ See `ein:notebook-open' for more information." (defun ein:notebook-pp (ewoc-data) (let ((path (ein:$node-path ewoc-data)) - (data (ein:$node-data ewoc-data)) - (inhibit-read-only t) - (buffer-undo-list t)) ; disable undo recording - ;; FIXME: Having `inhibit-read-only' and `buffer-undo-list' should - ;; make some of the same statements in ein-notebook/cell.el - ;; removable. These must be removed after careful review - ;; and maybe adding more tests. I will just let them be - ;; there since it is harmless. + (data (ein:$node-data ewoc-data))) (case (car path) - (cell (ein:cell-pp (cdr path) data)))) - (ein:notebook-empty-undo-maybe)) + (cell (ein:cell-pp (cdr path) data))))) ;;; Initialization. @@ -514,6 +506,7 @@ Prefixes are act same as the normal `yank' command." (ein:cell-insert-below base-cell cell)) (t (error (concat "`base-cell' is `nil' but ncells != 0. " "There is something wrong...")))) + (ein:notebook-empty-undo-maybe) (ein:cell-goto cell) (setf (ein:$notebook-dirty notebook) t)) cell)) @@ -544,6 +537,7 @@ when the prefix argument is given." (ein:cell-enter-first cell)))) (t (error (concat "`base-cell' is `nil' but ncells > 1. " "There is something wrong...")))) + (ein:notebook-empty-undo-maybe) (ein:cell-goto cell) (setf (ein:$notebook-dirty notebook) t)) cell)) @@ -576,6 +570,7 @@ directly." (let ((new (ein:cell-convert-inplace cell type))) (when (ein:codecell-p new) (oset new :kernel (ein:$notebook-kernel ein:notebook))) + (ein:notebook-empty-undo-maybe) (ein:cell-goto new))))) (defun ein:notebook-change-cell-type () @@ -599,7 +594,8 @@ Prompt will appear in the minibuffer." (when (ein:codecell-p new) (oset new :kernel (ein:$notebook-kernel ein:notebook))) (when level - (ein:cell-change-level new type)))))) + (ein:cell-change-level new type)) + (ein:notebook-empty-undo-maybe))))) (defun ein:notebook-split-cell-at-point (&optional no-trim) "Split cell at current position. Newlines at the splitting @@ -702,6 +698,7 @@ If prefix is given, merge current cell into previous cell." (defun ein:notebook-toggle-output (notebook cell) (ein:cell-toggle-output cell) + (ein:notebook-empty-undo-maybe) (setf (ein:$notebook-dirty notebook) t)) (defun ein:notebook-toggle-output-command () @@ -715,6 +712,7 @@ This does not alter the actual data stored in the cell." (mapc (lambda (c) (when (ein:codecell-p c) (ein:cell-set-collapsed c collapsed))) (ein:notebook-get-cells notebook)) + (ein:notebook-empty-undo-maybe) (setf (ein:$notebook-dirty notebook) t)) (defun ein:notebook-set-collapsed-all-command (&optional show) @@ -729,7 +727,8 @@ Do not clear input prompt when the prefix argument is given." (ein:notebook-with-cell #'ein:codecell-p (ein:cell-clear-output cell t t t) (unless preserve-input-prompt - (ein:cell-set-input-prompt cell)))) + (ein:cell-set-input-prompt cell)) + (ein:notebook-empty-undo-maybe))) (defun ein:notebook-clear-all-output-command (&optional preserve-input-prompt) "Clear output from all cells. @@ -740,7 +739,8 @@ Do not clear input prompts when the prefix argument is given." do (when (ein:codecell-p cell) (ein:cell-clear-output cell t t t) (unless preserve-input-prompt - (ein:cell-set-input-prompt cell)))) + (ein:cell-set-input-prompt cell)) + (ein:notebook-empty-undo-maybe))) (ein:log 'error "Not in notebook buffer!"))) diff --git a/tests/test-ein-notebook.el b/tests/test-ein-notebook.el index 9f02737..a3885eb 100644 --- a/tests/test-ein-notebook.el +++ b/tests/test-ein-notebook.el @@ -402,7 +402,7 @@ some text ;; Notebook undo -(ert-deftest ein:notebook-undo-after-execution-1-cell () +(defun eintest:notebook-undo-after-execution-1-cell () (with-current-buffer (eintest:notebook-make-empty) (ein:notebook-insert-cell-below-command) (let* ((text "print 'Hello World'") @@ -425,12 +425,15 @@ some text (funcall check-output) ;; Undo (should (equal (ein:cell-get-text cell) text)) - (undo) - (should (equal (ein:cell-get-text cell) "")) - ;; FIXME: Known bug. (it must succeed.) - (should-error (funcall check-output))))) + (if (eq ein:notebook-enable-undo 'full) + (undo) + (should-error (undo))) + (when (eq ein:notebook-enable-undo 'full) + (should (equal (ein:cell-get-text cell) "")) + ;; FIXME: Known bug. (it must succeed.) + (should-error (funcall check-output)))))) -(ert-deftest ein:notebook-undo-after-execution-2-cells () +(defun eintest:notebook-undo-after-execution-2-cells () (with-current-buffer (eintest:notebook-make-empty) (ein:notebook-insert-cell-below-command) (ein:notebook-insert-cell-above-command) @@ -464,11 +467,38 @@ some text ;; Undo (should (equal (ein:cell-get-text cell) text)) (should (equal (ein:cell-get-text next-cell) next-text)) - (undo) - (should (equal (ein:cell-get-text cell) text)) - ;; FIXME: Known bug. (these two must succeed.) - (should-error (should (equal (ein:cell-get-text next-cell) ""))) - (should-error (funcall check-output))))) + (if (eq ein:notebook-enable-undo 'full) + (undo) + (should-error (undo))) + (when (eq ein:notebook-enable-undo 'full) + (should (equal (ein:cell-get-text cell) text)) + ;; FIXME: Known bug. (these two must succeed.) + (should-error (should (equal (ein:cell-get-text next-cell) ""))) + (should-error (funcall check-output)))))) + +(ert-deftest ein:notebook-undo-after-execution-1-cell/no () + (let ((ein:notebook-enable-undo 'no)) + (eintest:notebook-undo-after-execution-1-cell))) + +(ert-deftest ein:notebook-undo-after-execution-1-cell/yes () + (let ((ein:notebook-enable-undo 'yes)) + (eintest:notebook-undo-after-execution-1-cell))) + +(ert-deftest ein:notebook-undo-after-execution-1-cell/full () + (let ((ein:notebook-enable-undo 'full)) + (eintest:notebook-undo-after-execution-1-cell))) + +(ert-deftest ein:notebook-undo-after-execution-2-cells/no () + (let ((ein:notebook-enable-undo 'no)) + (eintest:notebook-undo-after-execution-2-cells))) + +(ert-deftest ein:notebook-undo-after-execution-2-cells/yes () + (let ((ein:notebook-enable-undo 'yes)) + (eintest:notebook-undo-after-execution-2-cells))) + +(ert-deftest ein:notebook-undo-after-execution-2-cells/full () + (let ((ein:notebook-enable-undo 'full)) + (eintest:notebook-undo-after-execution-2-cells))) ;; Notebook mode From 6b5b54c965dea22dcb36afe4a2bcc6dd12533a8b Mon Sep 17 00:00:00 2001 From: Takafumi Arakaki Date: Sat, 16 Jun 2012 23:33:58 +0200 Subject: [PATCH 12/14] Trigger ein:notebook-empty-undo-maybe via event Previous tests were failed because undo was not reset after the execution callbacks modified the buffer. This is fixed by calling ein:notebook-empty-undo-maybe via event in all the callbacks. --- ein-cell.el | 16 ++++++++++------ ein-events.el | 4 +++- ein-notebook.el | 4 ++++ tests/test-ein-notebook.el | 3 +-- 4 files changed, 18 insertions(+), 9 deletions(-) diff --git a/ein-cell.el b/ein-cell.el index 7d006a3..7321606 100644 --- a/ein-cell.el +++ b/ein-cell.el @@ -794,12 +794,15 @@ Called from ewoc pretty printer via `ein:cell-insert-output'." (defmethod ein:cell--handle-execute-reply ((cell ein:codecell) content) (ein:cell-set-input-prompt cell (plist-get content :execution_count)) (ein:cell-running-set cell nil) - (ein:events-trigger - (oref cell :events) 'set_dirty.Notebook '(:value t))) + (let ((events (oref cell :events))) + (ein:events-trigger events 'set_dirty.Notebook '(:value t)) + (ein:events-trigger events 'maybe_reset_undo.Notebook))) (defmethod ein:cell--handle-set-next-input ((cell ein:codecell) text) - (ein:events-trigger - (oref cell :events) 'set_next_input.Notebook (list :cell cell :text text))) + (let ((events (oref cell :events))) + (ein:events-trigger events 'set_next_input.Notebook + (list :cell cell :text text)) + (ein:events-trigger events 'maybe_reset_undo.Notebook))) @@ -826,7 +829,7 @@ Called from ewoc pretty printer via `ein:cell-insert-output'." (plist-put json :traceback (plist-get content :traceback)))) (ein:cell-append-output cell json t) ;; (oset cell :dirty t) - )) + (ein:events-trigger (oref cell :events) 'maybe_reset_undo.Notebook))) (defun ein:output-area-convert-mime-types (json data) @@ -848,7 +851,8 @@ Called from ewoc pretty printer via `ein:cell-insert-output'." (ein:cell-clear-output cell (plist-get content :stdout) (plist-get content :stderr) - (plist-get content :other))) + (plist-get content :other)) + (ein:events-trigger (oref cell :events) 'maybe_reset_undo.Notebook)) ;;; Misc. diff --git a/ein-events.el b/ein-events.el index 1db3bc6..8c7aa70 100644 --- a/ein-events.el +++ b/ein-events.el @@ -62,7 +62,9 @@ When EVENT-TYPE is triggered on the event handler EVENTS, CALLBACK is called. CALLBACK must take two arguments: ARG as the first argument and DATA, which is passed via -`ein:events-trigger', as the second." +`ein:events-trigger', as the second. When calling the function, +current buffer is set to the configured buffer. `ein:events-new' +is used to configure the buffer." (assert (symbolp event-type)) (let* ((table (oref events :callbacks)) (cbs (gethash event-type table))) diff --git a/ein-notebook.el b/ein-notebook.el index 4f3823d..faa0249 100644 --- a/ein-notebook.el +++ b/ein-notebook.el @@ -356,6 +356,10 @@ See `ein:notebook-open' for more information." (setf (ein:$notebook-dirty notebook) (plist-get data :value))) notebook) + (ein:events-on events + 'maybe_reset_undo.Notebook + (lambda (&rest -ignore-) + (ein:notebook-empty-undo-maybe))) ;; Bind events for sub components: (mapc (lambda (cell) (oset cell :events (ein:$notebook-events notebook))) (ein:notebook-get-cells notebook)) diff --git a/tests/test-ein-notebook.el b/tests/test-ein-notebook.el index a3885eb..5ddc575 100644 --- a/tests/test-ein-notebook.el +++ b/tests/test-ein-notebook.el @@ -49,8 +49,7 @@ (with-current-buffer (ein:notebook-request-open-callback (ein:notebook-new "DUMMY-URL" notebook-id) :data (ein:json-read-from-string json-string)) - (let ((events (ein:events-new (current-buffer)))) - (setf (ein:$notebook-events ein:notebook) events) + (let ((events (ein:$notebook-events ein:notebook))) (setf (ein:$notebook-kernel ein:notebook) (ein:kernel-new 8888 "/kernels" events))) (current-buffer)))) From dc21cd2f386afe65cf724fbb1cf61ec704dbb7fa Mon Sep 17 00:00:00 2001 From: Takafumi Arakaki Date: Sun, 17 Jun 2012 00:19:00 +0200 Subject: [PATCH 13/14] Update README and doc Once this branch is merged, EIN will be "ready for release" state. --- README.rst | 5 +++-- doc/source/index.rst | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/README.rst b/README.rst index fce92ac..ecd2895 100644 --- a/README.rst +++ b/README.rst @@ -2,8 +2,9 @@ Emacs IPython Notebook (and more) =================================== -.. warning:: This is **very** early version. - Make sure you have backup! +.. note:: It is stable enough for my day to day work, but I can't + grantee the safety for your notebook data. So please make + sure you have backup. Screenshot ========== diff --git a/doc/source/index.rst b/doc/source/index.rst index 3d7b244..0481668 100644 --- a/doc/source/index.rst +++ b/doc/source/index.rst @@ -208,6 +208,7 @@ Notebook list Notebook ^^^^^^^^ +.. el:variable:: ein:notebook-enable-undo .. el:variable:: ein:notebook-discard-output-on-save .. el:variable:: ein:notebook-modes .. el:variable:: ein:notebook-kill-buffer-ask From 1ef27808c52834bc9fbe17e3ee41551f47015954 Mon Sep 17 00:00:00 2001 From: Takafumi Arakaki Date: Sun, 17 Jun 2012 01:12:13 +0200 Subject: [PATCH 14/14] Add dummy for maybe_reset_undo.Notebook in shared output --- ein-shared-output.el | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ein-shared-output.el b/ein-shared-output.el index 455b657..455a1af 100644 --- a/ein-shared-output.el +++ b/ein-shared-output.el @@ -109,8 +109,9 @@ ein:@shared-output))) (defun ein:shared-output-bind-events (events) - (ein:events-on events 'set_dirty.Notebook - (lambda (&rest ignore)))) + "Add dummy event handlers." + (ein:events-on events 'set_dirty.Notebook (lambda (&rest ignore))) + (ein:events-on events 'maybe_reset_undo.Notebook (lambda (&rest ignore)))) (defun ein:shared-output-get-cell () "Get the singleton shared output cell.