GNU bug report logs - #13801
[PATCH] Trivial fix for files.el

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 13801 in the body.
You can then email your comments to 13801 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

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

From: Xue Fuqiao <xfq.free <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Trivial fix for files.el
Date: Sun, 24 Feb 2013 14:33:32 +0800

I made a minor patch for files.el.  The patch does this:

1. Improve the doc string for `break-hardlink-on-save';

2. Remove an unnecessary `buffer-auto-save-file-name'.


*** trunk/lisp/files.el	2013-02-24 06:49:38.358835000 +0800
--- trunk/lisp/files.el.new	2013-02-24 14:23:28.681175258 +0800
*************** See also: `break-hardlink-on-save'."
*** 248,259 ****
    :group 'backup)
  
  (defcustom break-hardlink-on-save nil
!   "Non-nil means when saving a file that exists under several names
! \(i.e., has multiple hardlinks), break the hardlink associated with
! `buffer-file-name' and write to a new file, so that the other
! instances of the file are not affected by the save.
  
! If `buffer-file-name' refers to a symlink, do not break the symlink.
  
  Unlike `file-precious-flag', `break-hardlink-on-save' is not advisory.
  For example, if the directory in which a file is being saved is not
--- 248,262 ----
    :group 'backup)
  
  (defcustom break-hardlink-on-save nil
!   "This variable affect the relation between hardlink(s) and the file.
! Non-nil means when saving a file that exists under several names
! \(i.e., has multiple hardlinks), break the hardlink associated
! with the variable `buffer-file-name' and write to a new file, so
! that the other instances of the file are not affected by the
! save.
  
! If the variable`buffer-file-name' refers to a symlink, do not
! break the symlink.
  
  Unlike `file-precious-flag', `break-hardlink-on-save' is not advisory.
  For example, if the directory in which a file is being saved is not
*************** non-nil, it is called instead of rereadi
*** 5333,5339 ****
        (let* ((revert-buffer-in-progress-p t)
               (auto-save-p (and (not ignore-auto)
  			       (recent-auto-save-p)
- 			       buffer-auto-save-file-name
  			       (file-readable-p buffer-auto-save-file-name)
  			       (y-or-n-p
       "Buffer has been auto-saved recently.  Revert from auto-save file? ")))
--- 5336,5341 ----



*** trunk/lisp/ChangeLog	2013-02-24 06:49:38.358835000 +0800
--- trunk/lisp/ChangeLog.new	2013-02-24 14:27:41.126427073 +0800
***************
*** 1,7 ****
  2013-02-23  Peter Kleiweg  <p.c.j.kleiweg <at> rug.nl>
  
  	* progmodes/ps-mode.el (ps-mode-version): Bump to 1.1i.
! 	(ps-mode-octal-region): Use string-make-unibyte.
  
  2013-02-23  Glenn Morris  <rgm <at> gnu.org>
  
--- 1,12 ----
+ 2013-02-24  Xue Fuqiao  <xfq.free <at> gmail.com>
+ 
+ 	* files.el (break-hardlink-on-save): Doc fix.
+ 	(revert-buffer): Remove the unnecessary `buffer-auto-save-file-name'.
+ 
  2013-02-23  Peter Kleiweg  <p.c.j.kleiweg <at> rug.nl>
  
  	* progmodes/ps-mode.el (ps-mode-version): Bump to 1.1i.
! 	(ps-mode-octal-region): Use `string-make-unibyte'.
  
  2013-02-23  Glenn Morris  <rgm <at> gnu.org>
  


-- 
Best regards, Xue Fuqiao.
http://www.emacswiki.org/emacs/XueFuqiao

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Xue Fuqiao <xfq.free <at> gmail.com>
Cc: 13801 <at> debbugs.gnu.org
Subject: Re: bug#13801: [PATCH] Trivial fix for files.el
Date: Sun, 24 Feb 2013 17:06:39 +0200

> Date: Sun, 24 Feb 2013 14:33:32 +0800
> From: Xue Fuqiao <xfq.free <at> gmail.com>
> 
> *** trunk/lisp/files.el	2013-02-24 06:49:38.358835000 +0800
> --- trunk/lisp/files.el.new	2013-02-24 14:23:28.681175258 +0800
> *************** See also: `break-hardlink-on-save'."
> *** 248,259 ****
>     :group 'backup)
>   
>   (defcustom break-hardlink-on-save nil
> !   "Non-nil means when saving a file that exists under several names
> ! \(i.e., has multiple hardlinks), break the hardlink associated with
> ! `buffer-file-name' and write to a new file, so that the other
> ! instances of the file are not affected by the save.
>   
> ! If `buffer-file-name' refers to a symlink, do not break the symlink.
>   
>   Unlike `file-precious-flag', `break-hardlink-on-save' is not advisory.
>   For example, if the directory in which a file is being saved is not
> --- 248,262 ----
>     :group 'backup)
>   
>   (defcustom break-hardlink-on-save nil
> !   "This variable affect the relation between hardlink(s) and the file.
> ! Non-nil means when saving a file that exists under several names
> ! \(i.e., has multiple hardlinks), break the hardlink associated
> ! with the variable `buffer-file-name' and write to a new file, so
> ! that the other instances of the file are not affected by the
> ! save.

The first line of the doc string should concisely say what the option
does.  The old doc string did it, albeit not perfectly; the one you
suggest does not.

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

From: "Drew Adams" <drew.adams <at> oracle.com>
To: "'Eli Zaretskii'" <eliz <at> gnu.org>, "'Xue Fuqiao'" <xfq.free <at> gmail.com>
Cc: 13801 <at> debbugs.gnu.org
Subject: RE: bug#13801: [PATCH] Trivial fix for files.el
Date: Sun, 24 Feb 2013 09:47:44 -0800

> The first line of the doc string should concisely say what
> the option does.

Correct.

Including, if possible, what a nil or non-nil value does, if the option is
Boolean.  And the first line should (must) be a full sentence.

> The old doc string did it, albeit not perfectly;

Incorrect.  The first line did not do that at all.  At all.

> the one you suggest does not.

Correct.  My guess is that it was an attempt to provide a short full sentence
(good), but it does not say what the option does (not good).

What is important in both the old and new doc strings, and can serve as the
first line, is that the option does this (77 chars):

"Non-nil means break a hard link for the visited file and write to a new file."

If you feel that 77 chars is too much, you can drop "a" before "hard link" or
"the" before "visited file".  The rest of the doc string can clarify things
further (e.g., mentioning `buffer-file-name').

Whatever the wording chosen, the point is that non-nil means Emacs writes a new
file, bypassing any hard link for `buffer-file-name'.

In fact, I'm not sure about "break" here.  Is the effect that the hard link no
longer exists, or simply that it is ignored by Emacs when saving?  Depending on
the answer, the doc might need to be tweaked a little more.

The old first line was not even a complete sentence.  And the old first sentence
was four lines!

And unless I'm mistaken, the old first sentence was incorrect or at least
misleading regarding the condition where the option applies: "has multiple
hardlinks".

I think the correct criterion is simply having a hard link for the visited file
(`buffer-file-name'), regardless of whether there are any other hard links,
i.e., not necessarily multiple such.

It is not easy to write a short summary sentence, especially when English is not
your maternal language.  This is a welcome initiative from Fuqiao.  He clearly
takes an interest in the doc (as do Eli and Drew), and that is a fairly rare
resource.  Thank you.

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

From: Stephen Berman <stephen.berman <at> gmx.net>
To: "Drew Adams" <drew.adams <at> oracle.com>
Cc: 'Xue Fuqiao' <xfq.free <at> gmail.com>, 'Eli Zaretskii' <eliz <at> gnu.org>,
	13801 <at> debbugs.gnu.org
Subject: Re: bug#13801: [PATCH] Trivial fix for files.el
Date: Sun, 24 Feb 2013 22:45:41 +0100

On Sun, 24 Feb 2013 09:47:44 -0800 "Drew Adams" <drew.adams <at> oracle.com> wrote:

>> The first line of the doc string should concisely say what
>> the option does.
>
> Correct.
>
> Including, if possible, what a nil or non-nil value does, if the option is
> Boolean.  And the first line should (must) be a full sentence.
>
>> The old doc string did it, albeit not perfectly;
>
> Incorrect.  The first line did not do that at all.  At all.
>
>> the one you suggest does not.
>
> Correct.  My guess is that it was an attempt to provide a short full sentence
> (good), but it does not say what the option does (not good).
>
> What is important in both the old and new doc strings, and can serve as the
> first line, is that the option does this (77 chars):
>
> "Non-nil means break a hard link for the visited file and write to a new file."

To keep it under 68 characters (as per (elisp) Documentation Tips), how
about:

Non-nil means write visited file to a new file, breaking hard links.

Steve Berman

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

From: "Drew Adams" <drew.adams <at> oracle.com>
To: "'Stephen Berman'" <stephen.berman <at> gmx.net>
Cc: 'Xue Fuqiao' <xfq.free <at> gmail.com>, 'Eli Zaretskii' <eliz <at> gnu.org>,
	13801 <at> debbugs.gnu.org
Subject: RE: bug#13801: [PATCH] Trivial fix for files.el
Date: Sun, 24 Feb 2013 14:27:06 -0800

> > "Non-nil means break a hard link for the visited file and 
> write to a new file."
> 
> To keep it under 68 characters (as per (elisp) Documentation 
> Tips), how about:
> 
> Non-nil means write visited file to a new file, breaking hard links.

OK by me.  (I was thinking the guideline limit is more than 68.)

But that text suggests that a new file is always used.  Using "any hard link" or
"a hard link" is a bit better, suggesting that the new file thing is conditional
on there being a hard link.

Maybe this (66 chars)?

Non-nil means write new file if `buffer-file-name' is hard-linked.

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

From: Xue Fuqiao <xfq.free <at> gmail.com>
To: "Drew Adams" <drew.adams <at> oracle.com>
Cc: 'Eli Zaretskii' <eliz <at> gnu.org>, 'Stephen Berman' <stephen.berman <at> gmx.net>,
	13801 <at> debbugs.gnu.org
Subject: Re: bug#13801: [PATCH] Trivial fix for files.el
Date: Mon, 25 Feb 2013 06:47:26 +0800

On Sun, 24 Feb 2013 14:27:06 -0800
"Drew Adams" <drew.adams <at> oracle.com> wrote:

> > > "Non-nil means break a hard link for the visited file and 
> > write to a new file."
> > To keep it under 68 characters (as per (elisp) Documentation 
> > Tips), how about:
> > Non-nil means write visited file to a new file, breaking hard links.
> OK by me.  (I was thinking the guideline limit is more than 68.)
> But that text suggests that a new file is always used.  Using "any hard link" or
> "a hard link" is a bit better, suggesting that the new file thing is conditional
> on there being a hard link.
> Maybe this (66 chars)?
> Non-nil means write new file if `buffer-file-name' is hard-linked.

I think there are three problems (to me) about this version:

1. "write new file" is confusing.  I don't know what kind of thing will be write to which file;

2. `buffer-file-name' has both a function cell and a value cell, a little confusing;

2. `buffer-file-name' is a string (or returns a string), not a file, so the file cannot be hard-linked.

But I can't give a better version, sorry.

-- 
Best regards, Xue Fuqiao.
http://www.emacswiki.org/emacs/XueFuqiao

Message #25 received at 13801-done <at> debbugs.gnu.org (full text, mbox):

From: Chong Yidong <cyd <at> gnu.org>
To: Xue Fuqiao <xfq.free <at> gmail.com>
Cc: 13801-done <at> debbugs.gnu.org
Subject: Re: bug#13801: [PATCH] Trivial fix for files.el
Date: Tue, 17 Dec 2013 23:10:28 +0800

Xue Fuqiao <xfq.free <at> gmail.com> writes:

> I made a minor patch for files.el.  The patch does this:
>
> 1. Improve the doc string for `break-hardlink-on-save';

Thanks, I committed a slightly different fix.

> 2. Remove an unnecessary `buffer-auto-save-file-name'.
>
> - 			       buffer-auto-save-file-name
>   			       (file-readable-p buffer-auto-save-file-name)

This is not unnecessary.  If buffer-auto-save-file-name is nil, the next
line would signal an error, which is undesireable.

Previous Next

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