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

[Karl Fogel]

"Richard M. Stallman" <address@hidden> 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  <address@hidden>

        * 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