[PATCHv3 0/3] Add `notes' function, take three

[PATCHv3 0/3] Add `notes' function, take three

[Michal Nazarewicz]

All right, the third take for the `notes' function.  I've been running
this for a couple of days so I'm reasonably sure it won't blow up
anyone's computer. ;)

Change to kill-buffer allowed an advice to be replaced with a hook,
and after looking more closely at remember.el, I think notes might be
a good fit for it.

Stefan, are you still not convinced, and prefer to have it in ELPA
after all?

The second patch is actually unrelated, but something I've noticed
while playing with notes.

Michal Nazarewicz (3):
  `kill-buffer' runs query functions before checking buffer
    modification.
  `remember-append-to-file' appending to buffer bug fix
  Add `notes' function to store random notes across Emacs restarts.

 etc/NEWS                   |   8 ++
 lisp/ChangeLog             |  31 ++++++++
 lisp/startup.el            |   3 +-
 lisp/textmodes/remember.el | 177 ++++++++++++++++++++++++++++++++++++++++++---
 lisp/window.el             |   8 ++
 src/ChangeLog              |   8 ++
 src/buffer.c               |  24 +++---
 7 files changed, 234 insertions(+), 25 deletions(-)

-- 
1.8.3.1

[PATCH 1/3] `kill-buffer' runs query functions before checking buffer modification.

[Michal Nazarewicz]

* buffer.c (FKill_buffer): Run `kill-buffer-query-functions'
before checking whether buffer is modified.  This lets
`kill-buffer-query-functions' cancel killing of the buffer or save
its content before `kill-buffer' asks user the "Buffer %s
modified; kill anyway?" question.
---
 src/ChangeLog |  8 ++++++++
 src/buffer.c  | 24 ++++++++++++------------
 2 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/src/ChangeLog b/src/ChangeLog
index 4e52f5b..059c1d9 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,11 @@
+2013-06-21  Michal Nazarewicz  <mina86@mina86.com>
+
+	* buffer.c (FKill_buffer): Run `kill-buffer-query-functions'
+	before checking whether buffer is modified.  This lets
+	`kill-buffer-query-functions' cancel killing of the buffer or save
+	its content before `kill-buffer' asks user the "Buffer %s
+	modified; kill anyway?" question.
+
 2013-06-20  Paul Eggert  <eggert@cs.ucla.edu>
 
 	* syntax.c: Integer cleanups.
diff --git a/src/buffer.c b/src/buffer.c
index 08299da..066b31d 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -1734,18 +1734,6 @@ cleaning up all windows currently displaying the buffer to be killed. */)
   if (!BUFFER_LIVE_P (b))
     return Qnil;
 
-  /* Query if the buffer is still modified.  */
-  if (INTERACTIVE && !NILP (BVAR (b, filename))
-      && BUF_MODIFF (b) > BUF_SAVE_MODIFF (b))
-    {
-      GCPRO1 (buffer);
-      tem = do_yes_or_no_p (format2 ("Buffer %s modified; kill anyway? ",
-				     BVAR (b, name), make_number (0)));
-      UNGCPRO;
-      if (NILP (tem))
-	return Qnil;
-    }
-
   /* Run hooks with the buffer to be killed the current buffer.  */
   {
     ptrdiff_t count = SPECPDL_INDEX ();
@@ -1761,6 +1749,18 @@ cleaning up all windows currently displaying the buffer to be killed. */)
     if (NILP (tem))
       return unbind_to (count, Qnil);
 
+    /* Query if the buffer is still modified.  */
+    if (INTERACTIVE && !NILP (BVAR (b, filename))
+	&& BUF_MODIFF (b) > BUF_SAVE_MODIFF (b))
+      {
+        GCPRO1 (buffer);
+        tem = do_yes_or_no_p (format2 ("Buffer %s modified; kill anyway? ",
+				       BVAR (b, name), make_number (0)));
+	UNGCPRO;
+	if (NILP (tem))
+	  return unbind_to (count, Qnil);
+      }
+
     /* Then run the hooks.  */
     Frun_hooks (1, &Qkill_buffer_hook);
     unbind_to (count, Qnil);
-- 
1.8.3.1

[PATCH 2/3] `remember-append-to-file' appending to buffer bug fix

[Michal Nazarewicz]

* remember.el (remember-append-to-file): Function used
`find-buffer-visiting' to check whether a file visiting
`remember-data-file` existed but then `get-buffer-visiting' to
retrieve it.  However, the latter requires exact string match so
it could return nil even though the former found a buffer.
Storing result of the `find-buffer-visiting' fixes the issue (and
saves one lookup).
---
 lisp/ChangeLog             | 10 ++++++++++
 lisp/textmodes/remember.el | 25 ++++++++++++++-----------
 2 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/lisp/ChangeLog b/lisp/ChangeLog
index c79fcbd..d7a16a6 100644
--- a/lisp/ChangeLog
+++ b/lisp/ChangeLog
@@ -1,3 +1,13 @@
+2013-06-21  Michal Nazarewicz  <mina86@mina86.com>
+
+	* remember.el (remember-append-to-file): Function used
+	`find-buffer-visiting' to check whether a file visiting
+	`remember-data-file` existed but then `get-buffer-visiting' to
+	retrieve it.  However, the latter requires exact string match so
+	it could return nil even though the former found a buffer.
+	Storing result of the `find-buffer-visiting' fixes the issue (and
+	saves one lookup).
+
 2013-06-21  Leo Liu  <sdl.web@gmail.com>
 
 	* comint.el (comint-redirect-results-list-from-process): Fix
diff --git a/lisp/textmodes/remember.el b/lisp/textmodes/remember.el
index 5782f25..a14a34c 100644
--- a/lisp/textmodes/remember.el
+++ b/lisp/textmodes/remember.el
@@ -395,19 +395,22 @@ Subject: %s\n\n"
   "Remember, with description DESC, the given TEXT."
   (let ((text (buffer-string))
         (desc (remember-buffer-desc)))
-    (with-temp-buffer
-      (insert "\n" remember-leader-text (current-time-string)
-              " (" desc ")\n\n" text)
-      (if (not (bolp))
-          (insert "\n"))
-      (if (find-buffer-visiting remember-data-file)
-          (let ((remember-text (buffer-string)))
-            (set-buffer (get-file-buffer remember-data-file))
+    (let ((buf (find-buffer-visiting remember-data-file)))
+      (if buf
+          (with-current-buffer buf
             (save-excursion
               (goto-char (point-max))
-              (insert remember-text)
-              (when remember-save-after-remembering (save-buffer))))
-        (append-to-file (point-min) (point-max) remember-data-file)))))
+              (insert "\n" remember-leader-text (current-time-string)
+                      " (" desc ")\n\n" text)
+              (unless (bolp)
+                (insert "\n")))
+            (if remember-save-after-remembering (save-buffer)))
+        (with-temp-buffer
+          (insert "\n" remember-leader-text (current-time-string)
+                  " (" desc ")\n\n" text)
+          (unless (bolp)
+            (insert "\n"))
+          (append-to-file (point-min) (point-max) remember-data-file))))))
 
 (defun remember-region (&optional beg end)
   "Remember the data from BEG to END.
-- 
1.8.3.1

[PATCH 3/3] Add `notes' function to store random notes across Emacs restarts.

[Michal Nazarewicz]

You may think of it as a *scratch* buffer whose content is preserved.
In fact, it was designed as a replacement for *scratch* buffer and can
be used that way by setting `initial-buffer-choice' to 'notes an
`notes-buffer-name' to "*scratch*".  Without the second
change, *scratch* buffer will still be there for notes that do not
need to be preserved.

* notes.el: New file.
* remember.el (remember-data-file): Added :set callback to affect
notes buffer (if any).
(remember-append-to-file): Fixed buffer visiting a file checking.
Function used `find-buffer-visiting' to check whether such buffer
exists but then `get-buffer-visiting' to retrieve it.  However,
the latter requires exact string match so it could return nil even
though the former found a buffer.
(notes, toggle-notes): New functions for showing the notes buffer.
(notes-file, notes-recover-from-auto-save-file, notes-buffer-name)
(bury-notes-on-kill): New defcustoms for the notes buffer.
(notes--buffer, notes-map): New variables for the `notes' function.
(notes--custom-set-file, notes--insert-content)
(notes--kill-buffer-query): New helper functions.
* startup.el (initial-buffer-choice): Added notes to custom type.
* window.el (save-and-bury-buffer): New function doing what the
name says.
---
 etc/NEWS                   |   8 +++
 lisp/ChangeLog             |  21 +++++++
 lisp/startup.el            |   3 +-
 lisp/textmodes/remember.el | 152 ++++++++++++++++++++++++++++++++++++++++++++-
 lisp/window.el             |   8 +++
 5 files changed, 190 insertions(+), 2 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 475b4b2..ebe6589 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -87,6 +87,14 @@ simply disabling Transient Mark mode does the same thing.
 ** `initial-buffer-choice' can now specify a function to set up the
 initial buffer.
 
+** `notes' function creates a buffer whose content is saved on kill-emacs.
+You may think of it as a *scratch* buffer whose content is preserved.
+In fact, it was designed as a replacement for *scratch* buffer and can
+be used that way by setting `initial-buffer-choice' to 'notes an
+`notes-buffer-name' to "*scratch*".  Without the second
+change, *scratch* buffer will still be there for notes that do not
+need to be preserved.
+
 ** `write-region-inhibit-fsync' now defaults to t in batch mode.
 
 ** ACL support has been added.
diff --git a/lisp/ChangeLog b/lisp/ChangeLog
index d7a16a6..2a7caf8 100644
--- a/lisp/ChangeLog
+++ b/lisp/ChangeLog
@@ -1,5 +1,26 @@
 2013-06-21  Michal Nazarewicz  <mina86@mina86.com>
 
+	Add `notes' function to store random notes across Emacs restarts.
+	* notes.el: New file.
+	* remember.el (remember-data-file): Added :set callback to affect
+	notes buffer (if any).
+	(remember-append-to-file): Fixed buffer visiting a file checking.
+	Function used `find-buffer-visiting' to check whether such buffer
+	exists but then `get-buffer-visiting' to retrieve it.  However,
+	the latter requires exact string match so it could return nil even
+	though the former found a buffer.
+	(notes, toggle-notes): New functions for showing the notes buffer.
+	(notes-file, notes-recover-from-auto-save-file, notes-buffer-name)
+	(bury-notes-on-kill): New defcustoms for the notes buffer.
+	(notes--buffer, notes-map): New variables for the `notes' function.
+	(notes--custom-set-file, notes--insert-content)
+	(notes--kill-buffer-query): New helper functions.
+	* startup.el (initial-buffer-choice): Added notes to custom type.
+	* window.el (save-and-bury-buffer): New function doing what the
+	name says.
+
+2013-06-19  Michal Nazarewicz  <mina86@mina86.com>
+
 	* remember.el (remember-append-to-file): Function used
 	`find-buffer-visiting' to check whether a file visiting
 	`remember-data-file` existed but then `get-buffer-visiting' to
diff --git a/lisp/startup.el b/lisp/startup.el
index 77b2bce..7004015 100644
--- a/lisp/startup.el
+++ b/lisp/startup.el
@@ -53,7 +53,8 @@ or directory when no target file is specified."
 	  (const     :tag "Startup screen" nil)
 	  (directory :tag "Directory" :value "~/")
 	  (file      :tag "File" :value "~/.emacs")
-          (function  :tag "Function")
+	  (const     :tag "Notes buffer" notes)
+	  (function  :tag "Function")
 	  (const     :tag "Lisp scratch buffer" t))
   :version "24.4"
   :group 'initialization)
diff --git a/lisp/textmodes/remember.el b/lisp/textmodes/remember.el
index a14a34c..6b106c7 100644
--- a/lisp/textmodes/remember.el
+++ b/lisp/textmodes/remember.el
@@ -382,8 +382,12 @@ Subject: %s\n\n"
 ;; Remembering to plain files
 
 (defcustom remember-data-file (locate-user-emacs-file "notes" ".notes")
-  "The file in which to store unprocessed data."
+  "The file in which to store unprocessed data.
+When set via customize, visited file of the notes buffer (if it exists)
+might be changed.  This is only of importance if you are using `notes'."
   :type 'file
+  :set 'notes--custom-set-file
+  :initialize 'custom-initialize-default
   :group 'remember)
 
 (defcustom remember-leader-text "** "
@@ -554,4 +558,150 @@ the data away for latter retrieval, and possible indexing.
 \\{remember-mode-map}"
   (set-keymap-parent remember-mode-map nil))
 
+;; Notes buffer showing the notes:
+
+(defvar notes--buffer nil
+  "The notes buffer.")
+
+(defcustom notes-file t
+  "File to save notes in or t to use `remember-data-file'.
+When set via customize, visited file of the notes buffer \(if it exists)
+will be changed."
+  :type '(choice (const "Same as `remember-data-file'" t)
+                 (file  "Filename"))
+  :set 'notes--custom-set-file
+  :initialize 'custom-initialize-default
+  :group 'remember)
+
+(defcustom notes-recover-from-auto-save-file 'ask
+  "What to do if notes autosave file is newer than the notes file.
+
+t means to always recover content from auto-save file, 'ask means
+to ask user, and nil means never to recover auto-save file (which
+also disables `auto-save-mode' in the notes buffer.
+
+When set via customize, `auto-save-mode' will be enabled or disabled
+in the notes buffer according to this variable's value."
+  :type '(choice (const :tag "Always recover auto-save" t)
+                 (const :tag "Never recover auto-save" nil)
+                 (const :tag "Ask whether to recover auto-save" ask))
+  :set (lambda (symbol value)
+	 (set-default symbol value)
+	 (when (buffer-live-p notes--buffer)
+	   (with-current-buffer notes--buffer
+	     (auto-save-mode (if value 1 -1)))))
+  :group 'remember)
+
+(defcustom notes-buffer-name "*notes*"
+  "Name of the notes buffer.
+Setting it to *scratch* will hijack the *scratch* buffer for the
+purpose of storing notes."
+  :type 'string
+  :group 'remember)
+
+(defcustom initial-notes-major-mode t
+  "Major mode to set to notes buffer when it's created.
+If set to t will use the same mode as `initial-major-mode'."
+  :type '(choice (const    :tag "Same as `initial-major-mode'" t)
+		 (function :tag "Major mode" text-mode))
+  :group 'remember)
+
+(defcustom bury-notes-on-kill t
+  "Whether to bury notes buffer instead of killing."
+  :type 'boolean
+  :group 'remember)
+
+(defvar notes-map
+  (let ((map (make-sparse-keymap)))
+    (define-key map "\C-c\C-c" 'save-and-bury-buffer)
+    map))
+
+(defun notes--custom-set-file (symbol value)
+  (set-default symbol value)
+  (when (buffer-live-p notes--buffer)
+    (with-current-buffer notes--buffer
+      (setq buffer-file-name
+	    (expand-file-name
+	     (if (eq notes-file t) remember-data-file notes-file))))))
+
+;;;###autoload
+(defun notes ()
+  "Creates notes buffer and switches to it if called interactively.
+
+Name of the created buffer is taken from `notes-buffer-name' variable
+and if a buffer with that name already exist (but was not created by
+`notes' function), its content will be overwritten.
+\\<notes-map>
+`notes-map' is active in the notes buffer which by default contains
+only one \\[save-and-bury-buffer] binding which saves and buries the buffer.
+
+Function returns notes buffer.
+
+Notes buffer is meant for keeping random notes which you'd like to
+preserve across Emacs restarts.  The notes will be stored in the
+`notes-file'."
+  (interactive)
+  (unless (buffer-live-p notes--buffer)
+    (setq notes--buffer (get-buffer-create notes-buffer-name))
+    (with-current-buffer notes--buffer
+      (funcall (if (eq initial-notes-major-mode t)
+		   initial-major-mode
+		 initial-notes-major-mode))
+      (setq buffer-file-name
+	    (if (eq notes-file t) remember-data-file notes-file))
+      (setq buffer-save-without-query t)
+      (auto-save-mode (if notes-recover-from-auto-save-file 1 -1))
+      (add-hook 'kill-buffer-query-functions 'notes--kill-buffer-query)
+      (setq minor-mode-overriding-map-alist
+	    (cons (cons 'notes--buffer notes-map)
+		  minor-mode-overriding-map-alist))
+      (notes--insert-content)))
+  (when (called-interactively-p 'all)
+    (switch-to-buffer notes--buffer))
+  notes--buffer)
+
+;;;###autoload
+(defun toggle-notes ()
+  "Switches to notes buffer unless already there in which case buries it."
+  (interactive)
+  (if (eq (current-buffer) notes--buffer)
+      (bury-buffer)
+    (switch-to-buffer (notes))))
+
+(defun notes--insert-content ()
+  (let* ((have-file (file-readable-p buffer-file-name))
+	 (have-auto-save (and buffer-auto-save-file-name
+			      (file-readable-p buffer-auto-save-file-name))))
+    ;; If autosave is older, pretend it does not exist.
+    (and have-file
+	 have-auto-save
+	 (not (file-newer-than-file-p buffer-auto-save-file-name
+				       buffer-file-name))
+	 (setq have-auto-save nil))
+    ;; If user wants us to always recover, pretend there's no base file.
+    (and have-auto-save
+	 (eq t notes-recover-from-auto-save-file)
+	 (setq have-file nil))
+    ;; Ask user what to do.
+    (and have-file
+	 have-auto-save
+	 (if (y-or-n-p "Recover notes file? ")
+	     (setq have-file nil)
+	   (setq have-auto-save nil)))
+    (let ((file (cond (have-file buffer-file-name)
+		      (have-auto-save buffer-auto-save-file-name))))
+      (when file
+	(insert-file-contents file nil nil nil t)
+	(goto-char (point-min))
+	(set-buffer-modified-p nil)))))
+
+(defun notes--kill-buffer-query ()
+  (if (not (eq (current-buffer) notes--buffer))
+      t
+    (when (buffer-modified-p)
+      (save-buffer))
+    (if bury-notes-on-kill
+	(bury-buffer)
+      t)))
+
 ;;; remember.el ends here
diff --git a/lisp/window.el b/lisp/window.el
index 5b00198..7eda7ea 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -3429,6 +3429,14 @@ displayed there."
     ;; Always return nil.
     nil))
 
+(defun save-and-bury-buffer ()
+  "Saves and buries current buffer.
+Buffer is saved only if `buffer-modified-p' returns non-nil."
+  (interactive)
+  (when (buffer-modified-p)
+    (save-buffer))
+  (bury-buffer))
+
 (defun unbury-buffer ()
   "Switch to the last buffer in the buffer list."
   (interactive)
-- 
1.8.3.1

Re: [PATCHv3 0/3] Add `notes' function, take three

[Stefan Monnier]

> Change to kill-buffer allowed an advice to be replaced with a hook,

OK

> and after looking more closely at remember.el, I think notes might be
> a good fit for it.

Indeed.  Why even define a separate notes-file?
And why not use (find-file-noselect notes-file), maybe followed by
a rename-buffer?

> Stefan, are you still not convinced, and prefer to have it in ELPA
> after all?

No, I think remember.el is a good place for it.  It would also be good
to use the "remember-" prefix.

> The second patch is actually unrelated, but something I've noticed
> while playing with notes.

Good point.  I think we should turn get-file-buffer into an alias for
find-buffer-visiting.


        Stefan

Re: [PATCHv3 0/3] Add `notes' function, take three

[Michal Nazarewicz]

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

On Fri, Jun 21 2013, Stefan Monnier wrote:
>> Change to kill-buffer allowed an advice to be replaced with a hook,
>
> OK
>
>> and after looking more closely at remember.el, I think notes might be
>> a good fit for it.
>
> Indeed.  Why even define a separate notes-file?

I wanted to keep it as an option for people who would like to use
remember to store data in notes file which then gets processed somehow
(I've never used remember before now, so I'm not sure what that could
involve) while use notes for some random data.  As far as *I'm*
concerned, I can leave w/o notes-file and just remember-data-file.

> And why not use (find-file-noselect notes-file), maybe followed by
> a rename-buffer?

I'll take a look at it.

>> Stefan, are you still not convinced, and prefer to have it in ELPA
>> after all?
>
> No, I think remember.el is a good place for it.  It would also be good
> to use the "remember-" prefix.

Will do.

>> The second patch is actually unrelated, but something I've noticed
>> while playing with notes.
>
> Good point.  I think we should turn get-file-buffer into an alias for
> find-buffer-visiting.

$ grep find-buffer-visiting $(grep -Rl get-file-buffer **/*.el) |wc -l
43

so it seems that in most cases code sticks to either of the two
functions, and some of the above occurrences are XEmacs compatibility
functions, comments, and situations where it would seem that the code
could be easily converted to stick to find-buffer-visiting.

So yeah, I guess it is doable.

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--

[-- Attachment #2.1: Type: text/plain, Size: 0 bytes --]



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

Re: [PATCHv3 0/3] Add `notes' function, take three

[Stefan Monnier]

>> Indeed.  Why even define a separate notes-file?
> I wanted to keep it as an option for people who would like to use
> remember to store data in notes file which then gets processed somehow
> (I've never used remember before now, so I'm not sure what that could
> involve) while use notes for some random data.  As far as *I'm*
> concerned, I can leave w/o notes-file and just remember-data-file.

Let's keep things simple rather than try and accommodate
a hypothetical situation, especially since, if the needs arises, it's
still not that hard for the user to set things up the way she likes.


        Stefan