Re: [Emacs-commit] emacs/lisp saveplace.el

Re: [Emacs-commit] emacs/lisp saveplace.el

[Karl Fogel]

"Richard M. Stallman" <rms@gnu.org> writes:
> Log message:
> 	(save-place-alist-to-file): Write the file using write-region.
>
> Index: lisp/saveplace.el
> ===================================================================
> RCS file: /cvsroot/emacs/emacs/lisp/saveplace.el,v
> retrieving revision 1.31
> retrieving revision 1.32
> diff -u -r1.31 -r1.32
> --- lisp/saveplace.el	23 Apr 2005 16:40:24 -0000	1.31
> +++ lisp/saveplace.el	29 May 2005 08:36:26 -0000	1.32
> @@ -222,7 +222,8 @@
>                (t
>                 t))))
>  	(condition-case nil
> -	    (write-file file)
> +	    ;; Don't use write-file; we don't want this buffer to visit it.
> +	    (write-region (point-min) (point-max) file)
>  	  (file-error (message "Can't write %s" file)))
>          (kill-buffer (current-buffer))
>          (message "Saving places to %s...done" file)))))

What were the bad effects of using 'write-file'?

I ask because I'm wondering if the same change should be made to
'bookmark-write-file' in bookmark.el.

In fact, maybe there should be a bigger change to bookmark-write-file.
Instead of using find-file-noselect, it could just work the way
save-place-alist-to-file does: switch to a buffer, clear the buffer,
insert the new data, and write it out to the destination.  That is:

   2005-05-29  Karl Fogel  <kfogel@red-bean.com>

	* bookmark.el

        (bookmark-write-file): Don't visit the destination file, just
        write the data to it using write-region.  This is similar to
        revision 1.32 of saveplace.el, but with an additional change
        to avoid visiting the file in the first place.

   Index: lisp/bookmark.el
   ===================================================================
   RCS file: /cvsroot/emacs/emacs/lisp/bookmark.el,v
   retrieving revision 1.73
   diff -u -r1.73 bookmark.el
   --- lisp/bookmark.el	26 May 2005 12:28:39 -0000	1.73
   +++ lisp/bookmark.el	30 May 2005 02:17:40 -0000
   @@ -1340,12 +1340,11 @@
        (save-window-excursion
          (if (>= baud-rate 9600)
              (message "Saving bookmarks to file %s..." file))
   -      (set-buffer (let ((enable-local-variables nil))
   -                    (find-file-noselect file)))
   +      (set-buffer (get-buffer-create " *Bookmarks*"))
          (goto-char (point-min))
   +      (delete-region (point-min) (point-max))
          (let ((print-length nil)
    	    (print-level nil))
   -	(delete-region (point-min) (point-max))
    	(bookmark-insert-file-format-version-stamp)
    	(pp bookmark-alist (current-buffer))
    	(let ((version-control
   @@ -1356,7 +1355,7 @@
    		(t
    		 t))))
              (condition-case nil
   -              (write-file file)
   +              (write-region file)
                (file-error (message "Can't write %s" file)))
    	  (kill-buffer (current-buffer))
    	  (if (>= baud-rate 9600)
   
   
Does this patch look sane?

-Karl

Re: [Emacs-commit] emacs/lisp saveplace.el

[Richard Stallman]

    What were the bad effects of using 'write-file'?

It made saveplace act as if the user had visited this file,
so that the next session would restore the place.

    I ask because I'm wondering if the same change should be made to
    'bookmark-write-file' in bookmark.el.

If bookmark.el currently visits the file when reading it,
changing the way it writes the file would not alter the outcome.

    In fact, maybe there should be a bigger change to bookmark-write-file.
    Instead of using find-file-noselect, it could just work the way
    save-place-alist-to-file does: switch to a buffer, clear the buffer,
    insert the new data, and write it out to the destination.  That is:

How often do users visit the bookmark file?  If they do this commonly,
then it is better to visit the file.  If they do this quite rarely,
then could be is better to work like save-place-alist-to-file.

Another alternative, which could be best if you want Emacs to make backups
of the bookmark file in the usual way, is to visit the file normally,
but kill the buffer after saving it, unless the buffer existed previously.