GNU bug report logs - #12507
24.2.50; `bookmark-write-file': use `write-file', not `write-region', to get backups

Previous Next

Full log

Message #138 received at 12507 <at> debbugs.gnu.org (full text, mbox):

From: Karl Fogel <kfogel <at> red-bean.com>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, Chong Yidong <cyd <at> gnu.org>,
 12507 <at> debbugs.gnu.org
Subject: Re: bug#12507: [debbugs-tracker] Processed: severity 12507 wishlist
Date: Wed, 23 Sep 2020 01:41:52 -0500

I've just been deep-diving into this again.  I'd like feedback on this general plan for a solution:

1) In files.el: define a new function `back-up-file' [a] that takes a
   file name as argument and does most of what `backup-buffer' currently
   does -- basically, everything from the second `when' on down to the
   end of the function.

2) Change `backup-buffer' to use the new function `back-up-file'.

   In other words, we abstract out most of the Emacs file backup
   functionality to this new function `back-up-file' so we can call it
   in (3) below, passing a file name argument.

3) In bookmark.el, in `bookmark-write-file', call `back-up-file' right
   before the `write-region' call.  (Note that the ambient value of
   `version-control' will already be correct at this point.)

Before I make the patch, I wanted to propose the general idea here and see if it seems okay to others.

This way we would still use `write-region' and avoid visiting the file.  Even though we don't know exactly why Richard made the original motivating change (commit cfde584f6d, see [b] below), there was some problem with `write-file' -- so we shouldn't just go back to that, because a regression would be likely.

Thoughts on this plan?

Best regards,
-Karl

[a] Yes, I know I've inserted a hyphen into "back-up" in that function
    name.  That's because in English "back up" is a compound verb,
    whereas "backup" as one word is a noun.  The existing function name
    "backup-buffer" is thus grammatically suspect, but I'm not proposing
    to fix that here; I'm just trying to avoid repeating the confusion.
    If folks would prefer "backup-file" for consistency, I could do
    that, or maybe just find another name.  The details of the function
    names are not the point here -- I'm just looking for feedback on the
    general plan.

[b] Commit cfde584f6d:

    ===============================================
    commit cfde584f6d25db8c0ba229ebb85fba60e99eb1af
    Author:     Richard M. Stallman <rms <at> gnu.org>
    AuthorDate: Sun May 29 08:36:26 2005 +0000
    Commit:     Richard M. Stallman <rms <at> gnu.org>
    CommitDate: Sun May 29 08:36:26 2005 +0000
    
        (save-place-alist-to-file): Write the file using write-region.
    
    M	lisp/saveplace.el
    ===============================================
    --- lisp/saveplace.el
    +++ lisp/saveplace.el
    @@ -222,7 +222,8 @@ save-place-alist-to-file
                   (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)))))
    ===============================================

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.