Defcustom write-file-functions and write-contents-functions?

Defcustom write-file-functions and write-contents-functions?

[Simon Josefsson]

Shouldn't these two variables be customizable?  I add copyright-update
to w-f-f, as per the comment in copyright.el.  However, while making
this patch, it occurred to me that adding copyright-update to
write-contents-functions is probably more appropriate, so this also
change fixed the copyright.el commentary.  Ok to install?

2003-12-31  Simon Josefsson  <jas@extundo.com>

	* files.el (write-file): New defgroup.
	(write-file-functions): Customize, add to write-file defgroup.
	(write-contents-functions): Likewise.

	* emacs-lisp/copyright.el: Use write-contents-functions instead of
	write-file-functions.

Index: files.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/files.el,v
retrieving revision 1.673
diff -u -p -u -w -r1.673 files.el
--- files.el	29 Dec 2003 19:14:03 -0000	1.673
+++ files.el	31 Dec 2003 14:36:55 -0000
@@ -38,6 +38,9 @@
   "Finding files."
   :group 'files)
 
+(defgroup write-file nil
+  "Writing files."
+  :group 'files)
 
 (defcustom delete-auto-save-files t
   "*Non-nil means delete auto-save file when a buffer is saved or killed.
@@ -366,7 +369,7 @@ functions are called."
 (defvaralias 'find-file-hooks 'find-file-hook)
 (make-obsolete-variable 'find-file-hooks 'find-file-hook "21.4")
 
-(defvar write-file-functions nil
+(defcustom write-file-functions nil
   "List of functions to be called before writing out a buffer to a file.
 If one of them returns non-nil, the file is considered already written
 and the rest are not called.
@@ -375,7 +378,10 @@ So any buffer-local binding of this vari
 the visited file name with \\[set-visited-file-name], but not when you
 change the major mode.
 
-See also `write-contents-functions'.")
+See also `write-contents-functions'."
+  :type 'hook
+  :options '(copyright-update)
+  :group 'write-file)
 (put 'write-file-functions 'permanent-local t)
 (defvaralias 'write-file-hooks 'write-file-functions)
 (make-obsolete-variable 'write-file-hooks 'write-file-functions "21.4")
@@ -385,7 +391,7 @@ See also `write-contents-functions'.")
 (put 'local-write-file-hooks 'permanent-local t)
 (make-obsolete-variable 'local-write-file-hooks 'write-file-functions "21.4")
 
-(defvar write-contents-functions nil
+(defcustom write-contents-functions nil
   "List of functions to be called before writing out a buffer to a file.
 If one of them returns non-nil, the file is considered already written
 and the rest are not called.
@@ -395,7 +401,10 @@ buffer's contents, not to the particular
 `set-visited-file-name' does not clear this variable; but changing the
 major mode does clear it.
 
-See also `write-file-functions'.")
+See also `write-file-functions'."
+  :type 'hook
+  :options '(copyright-update)
+  :group 'write-file)
 (make-variable-buffer-local 'write-contents-functions)
 (defvaralias 'write-contents-hooks 'write-contents-functions)
 (make-obsolete-variable 'write-contents-hooks 'write-contents-functions "21.4")
Index: copyright.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/emacs-lisp/copyright.el,v
retrieving revision 1.43
diff -u -p -u -w -r1.43 copyright.el
--- copyright.el	1 Sep 2003 15:45:20 -0000	1.43
+++ copyright.el	31 Dec 2003 14:36:29 -0000
@@ -27,7 +27,8 @@
 
 ;; Allows updating the copyright year and above mentioned GPL version manually
 ;; or when saving a file.
-;; Do (add-hook 'write-file-functions 'copyright-update).
+;; Do (add-hook 'write-contents-functions 'copyright-update), or use
+;; M-x customize-variable RET write-contents-functions RET.
 
 ;;; Code:

Re: Defcustom write-file-functions and write-contents-functions?

[Luc Teirlinck]

Do you really want to make those two functions customizable through
Custom?  They seem to be intended to be used by programs.  They are
tricky.  They are not normal hooks.  The order matters a lot.  If a
user naively adds a function that returns a non-nil value, big
surprises may result.  Not something one might want the average user
to start playing apprentice sorcerer with.  (_Any_ Lisp variable is
customizable by a sophisticated user by writing Lisp code in .emacs or
files loaded by .emacs.)  Custom sets _global_ values and these two
hooks are intended to be file-specific and hence buffer-local.

I do not know how you envision the average user to use these
defcustoms.  Would the customizability you want to achieve not be
better served by adding a defcustom for a new no-tricks _normal_ hook,
say `before_save_hook', called before the other two?  (If there
already would be a hook of this type, then it should be documented in
(elisp)Saving Buffers.)

Sincerely,

Luc.

Re: Defcustom write-file-functions and write-contents-functions?

[Simon Josefsson]

Luc Teirlinck <teirllm@dms.auburn.edu> writes:

> Do you really want to make those two functions customizable through
> Custom?  They seem to be intended to be used by programs.

The example I gave, copyright-update, appear to be intended for users.

> They are tricky.  They are not normal hooks.  The order matters a
> lot.  If a user naively adds a function that returns a non-nil
> value, big surprises may result.  Not something one might want the
> average user to start playing apprentice sorcerer with.  (_Any_ Lisp
> variable is customizable by a sophisticated user by writing Lisp
> code in .emacs or files loaded by .emacs.)  Custom sets _global_
> values and these two hooks are intended to be file-specific and
> hence buffer-local.

I'm not sure I'm convinced; users can mess up their emacs environment
by customizing many variables.  At least with this variable, if the
user makes a mistake, she can revert the value without restarting
emacs, unlike e.g. max-specpdl-size or max-lisp-eval-depth.  And
editing away mistakes is rather easy in the custom buffer.

I believe it is better to encourage users to use custom because it
reduces typos and elisp quotation errors.  If these variables were
installed, the user could customize any of the variable and click on
the click box for `copyright-update', instead of typing (add-hook
'write-file-functions 'copyright-update) and risking making a typo,
forcing here to use remove-hook manually on the typo.

(An unrelated example: a non-negligible number of the posts regarding
setting up SMTP for authentication in Emacs contain elisp quotation
problems, such as using "25" instead of 25, or '("foo" 'bar).  If
those users used the custom interface instead, they wouldn't be able
to make those typos.  I don't know why they don't use custom, but
perhaps we simply don't encourage users to do so enough.)

> I do not know how you envision the average user to use these
> defcustoms.

My primary example is copyright-update.  It can be added to
write-file-functions or (probably better) write-contents-functions.

When the user is a developer, I even imagine this to be something even
an average user would want (if she know about the copyright.el
package).

> Would the customizability you want to achieve not be
> better served by adding a defcustom for a new no-tricks _normal_ hook,
> say `before_save_hook', called before the other two?  (If there
> already would be a hook of this type, then it should be documented in
> (elisp)Saving Buffers.)

This could work too.  My reason for choosing w-f-f was that
copyright.el suggested it, and I want to use custom to enable that
package.

Re: Defcustom write-file-functions and write-contents-functions?

[Luc Teirlinck]

Simon Josefsson wrote:

   The example I gave, copyright-update, appear to be intended for users.

This does not contradict what I said.  From its documentation,
`write-contents-functions' seems to be a mode-specific way to save
files, `write-file-functions' appears to be a file-specific way to do
so.  Custom can not really handle either buffer-local or file-local
customization.  Personally, I believe you need a new _normal_ hook,
intended to be called unconditionally.

   This could work too.  My reason for choosing w-f-f was that
   copyright.el suggested it, and I want to use custom to enable that
   package.

That suggestion was wrong, because w-f-f is _not_ unconditionally run.
It is the third of a bunch of hooks called in an `or' form in
`basic-save-buffer'.  If any of the two prior hooks in the or form
takes care of saving the file, w-f-f is _not_ run.  (I personally
believe that the docstring and the definition in he Elisp manual
should mention this.)  Your patch replaced it with w-c-f.  This is
better, because that is the first hook in the `or' form.  But major
modes would seem free to override anything the user customized using
your defcustom, because the variable is intended to be set
buffer-locally by major modes.

Again, I personally believe that for what you want to do, one needs a
new normal hook, to be called unconditionally, _before_ the `or' form
in `basic-save-buffer'.

Sincerely,

Luc.

Re: Defcustom write-file-functions and write-contents-functions?

[Simon Josefsson]

You convinced me, thanks for the feedback.  How about this patch,
instead?

2003-12-31  Simon Josefsson  <jas@extundo.com>

	* files.el (before-save-hook): Add.
	(basic-save-buffer): Use before-save-hook.

	* emacs-lisp/copyright.el: Use before-save-hook instead of
	write-file-functions.

Index: lisp/files.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/files.el,v
retrieving revision 1.673
diff -u -p -r1.673 files.el
--- lisp/files.el	29 Dec 2003 19:14:03 -0000	1.673
+++ lisp/files.el	31 Dec 2003 19:51:10 -0000
@@ -2990,6 +2990,12 @@ the last real save, but optional arg FOR
 (defvar auto-save-hook nil
   "Normal hook run just before auto-saving.")
 
+(defcustom before-save-hook nil
+  "Normal hook that is run before a buffer is saved to its file."
+  :options '(copyright-update)
+  :type 'hook
+  :group 'files)
+
 (defcustom after-save-hook nil
   "Normal hook that is run after a buffer is saved to its file."
   :options '(executable-make-buffer-file-executable-if-script-p)
@@ -3012,7 +3018,8 @@ in such cases.")
 The hooks `write-contents-functions' and `write-file-functions' get a chance
 to do the job of saving; if they do not, then the buffer is saved in
 the visited file file in the usual way.
-After saving the buffer, this function runs `after-save-hook'."
+Before and after saving the buffer, this function runs
+`before-save-hook' and `after-save-hook', respectively."
   (interactive)
   (save-current-buffer
     ;; In an indirect buffer, save its base buffer instead.
@@ -3068,6 +3075,7 @@ After saving the buffer, this function r
 		     (insert ?\n))))
 	    ;; Support VC version backups.
 	    (vc-before-save)
+	    (run-hooks 'before-save-hook)
 	    (or (run-hook-with-args-until-success 'write-contents-functions)
 		(run-hook-with-args-until-success 'local-write-file-hooks)
 		(run-hook-with-args-until-success 'write-file-functions)
Index: lisp/emacs-lisp/copyright.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/emacs-lisp/copyright.el,v
retrieving revision 1.43
diff -u -p -r1.43 copyright.el
--- lisp/emacs-lisp/copyright.el	1 Sep 2003 15:45:20 -0000	1.43
+++ lisp/emacs-lisp/copyright.el	31 Dec 2003 19:51:10 -0000
@@ -27,7 +27,8 @@
 
 ;; Allows updating the copyright year and above mentioned GPL version manually
 ;; or when saving a file.
-;; Do (add-hook 'write-file-functions 'copyright-update).
+;; Do (add-hook 'before-save-hook 'copyright-update), or use
+;; M-x customize-variable RET before-save-hook RET.
 
 ;;; Code:

Re: Defcustom write-file-functions and write-contents-functions?

[Luc Teirlinck]

Simon Josefsson wrote:

   How about this patch, instead?

To me, that looks indeed better.

Sincerely,

Luc.

Re: Defcustom write-file-functions and write-contents-functions?

[Eli Zaretskii]

> From: Simon Josefsson <jas@extundo.com>
> Date: Wed, 31 Dec 2003 20:56:21 +0100
> 
> You convinced me, thanks for the feedback.  How about this patch,
> instead?
> 
> 2003-12-31  Simon Josefsson  <jas@extundo.com>
> 
> 	* files.el (before-save-hook): Add.

This addition should be followed by 2 documentation changes: (1) to
the ELisp manual, and (2) to etc/NEWS.

TIA

Re: Defcustom write-file-functions and write-contents-functions?

[Simon Josefsson]

Eli Zaretskii <eliz@elta.co.il> writes:

>> From: Simon Josefsson <jas@extundo.com>
>> Date: Wed, 31 Dec 2003 20:56:21 +0100
>> 
>> You convinced me, thanks for the feedback.  How about this patch,
>> instead?
>> 
>> 2003-12-31  Simon Josefsson  <jas@extundo.com>
>> 
>> 	* files.el (before-save-hook): Add.
>
> This addition should be followed by 2 documentation changes: (1) to
> the ELisp manual, and (2) to etc/NEWS.

Yes, although I waited with that until people said it was a good idea
to install it.  Maybe that time is now?  If so:

Index: etc/NEWS
===================================================================
RCS file: /cvsroot/emacs/emacs/etc/NEWS,v
retrieving revision 1.882
diff -u -p -r1.882 NEWS
--- etc/NEWS	30 Dec 2003 23:39:57 -0000	1.882
+++ etc/NEWS	1 Jan 2004 12:21:03 -0000
@@ -1,5 +1,5 @@
 GNU Emacs NEWS -- history of user-visible changes.  2003-05-21
-Copyright (C) 1999, 2000, 2001, 2002, 2003 Free Software Foundation, Inc.
+Copyright (C) 1999, 2000, 2001, 2002, 2003, 2004 Free Software Foundation, Inc.
 See the end for copying conditions.
 
 Please send Emacs bug reports to bug-gnu-emacs@gnu.org.
@@ -1736,6 +1736,11 @@ a match if part of it has a read-only pr
 configuration files.
 \f
 * Lisp Changes in Emacs 21.4
+
+** The new hook `before-save-hook' is invoked by `basic-save-buffer'
+before saving buffers.  This allows packages to perform various final
+tasks, for example; it can be used by the copyright package to make
+sure saved files have the current year in any copyright headers.
 
 ** The function `insert-for-yank' now supports strings where the
 `yank-handler' property does not span the first character of the
Index: lisp/ChangeLog
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/ChangeLog,v
retrieving revision 1.5573
diff -u -p -r1.5573 ChangeLog
--- lisp/ChangeLog	31 Dec 2003 02:46:07 -0000	1.5573
+++ lisp/ChangeLog	1 Jan 2004 12:21:05 -0000
@@ -1,3 +1,11 @@
+2003-12-31  Simon Josefsson  <jas@extundo.com>
+
+	* files.el (before-save-hook): Add.
+	(basic-save-buffer): Use before-save-hook.
+
+	* emacs-lisp/copyright.el: Use before-save-hook instead of
+	write-file-functions.
+
 2003-12-31  John Paul Wallington  <jpw@gnu.org>
 
 	* bindings.el (completion-ignored-extensions): Add .pfsl.
Index: lisp/files.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/files.el,v
retrieving revision 1.673
diff -u -p -r1.673 files.el
--- lisp/files.el	29 Dec 2003 19:14:03 -0000	1.673
+++ lisp/files.el	1 Jan 2004 12:21:06 -0000
@@ -2990,6 +2990,12 @@ the last real save, but optional arg FOR
 (defvar auto-save-hook nil
   "Normal hook run just before auto-saving.")
 
+(defcustom before-save-hook nil
+  "Normal hook that is run before a buffer is saved to its file."
+  :options '(copyright-update)
+  :type 'hook
+  :group 'files)
+
 (defcustom after-save-hook nil
   "Normal hook that is run after a buffer is saved to its file."
   :options '(executable-make-buffer-file-executable-if-script-p)
@@ -3012,7 +3018,8 @@ in such cases.")
 The hooks `write-contents-functions' and `write-file-functions' get a chance
 to do the job of saving; if they do not, then the buffer is saved in
 the visited file file in the usual way.
-After saving the buffer, this function runs `after-save-hook'."
+Before and after saving the buffer, this function runs
+`before-save-hook' and `after-save-hook', respectively."
   (interactive)
   (save-current-buffer
     ;; In an indirect buffer, save its base buffer instead.
@@ -3068,6 +3075,7 @@ After saving the buffer, this function r
 		     (insert ?\n))))
 	    ;; Support VC version backups.
 	    (vc-before-save)
+	    (run-hooks 'before-save-hook)
 	    (or (run-hook-with-args-until-success 'write-contents-functions)
 		(run-hook-with-args-until-success 'local-write-file-hooks)
 		(run-hook-with-args-until-success 'write-file-functions)
Index: lisp/emacs-lisp/copyright.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/emacs-lisp/copyright.el,v
retrieving revision 1.43
diff -u -p -r1.43 copyright.el
--- lisp/emacs-lisp/copyright.el	1 Sep 2003 15:45:20 -0000	1.43
+++ lisp/emacs-lisp/copyright.el	1 Jan 2004 12:21:06 -0000
@@ -27,7 +27,8 @@
 
 ;; Allows updating the copyright year and above mentioned GPL version manually
 ;; or when saving a file.
-;; Do (add-hook 'write-file-functions 'copyright-update).
+;; Do (add-hook 'before-save-hook 'copyright-update), or use
+;; M-x customize-variable RET before-save-hook RET.
 
 ;;; Code:
 
Index: lispref/ChangeLog
===================================================================
RCS file: /cvsroot/emacs/emacs/lispref/ChangeLog,v
retrieving revision 1.140
diff -u -p -r1.140 ChangeLog
--- lispref/ChangeLog	1 Jan 2004 04:20:43 -0000	1.140
+++ lispref/ChangeLog	1 Jan 2004 12:21:07 -0000
@@ -1,3 +1,8 @@
+2004-01-01  Simon Josefsson  <jas@extundo.com>
+
+	* hooks.texi (Standard Hooks): Add before-save-hook.
+	* files.texi (Saving Buffers): Likewise.
+
 2004-01-01  Miles Bader  <miles@gnu.org>
 
 	* display.texi (Buttons): New section.
Index: lispref/files.texi
===================================================================
RCS file: /cvsroot/emacs/emacs/lispref/files.texi,v
retrieving revision 1.59
diff -u -p -r1.59 files.texi
--- lispref/files.texi	29 Dec 2003 20:28:40 -0000	1.59
+++ lispref/files.texi	1 Jan 2004 12:21:07 -0000
@@ -1,6 +1,6 @@
 @c -*-texinfo-*-
 @c This is part of the GNU Emacs Lisp Reference Manual.
-@c Copyright (C) 1990, 1991, 1992, 1993, 1994, 1995, 1998, 1999
+@c Copyright (C) 1990, 1991, 1992, 1993, 1994, 1995, 1998, 1999, 2004
 @c   Free Software Foundation, Inc.
 @c See the file elisp.texi for copying conditions.
 @setfilename ../info/files
@@ -408,6 +408,13 @@ major modes, as buffer-local bindings fo
 
 This variable automatically becomes buffer-local whenever it is set;
 switching to a new major mode always resets this variable.
+@end defvar
+
+@defvar before-save-hook
+This normal hook runs before a buffer has been saved in its visited
+file.  One use of this hook is for the Copyright package; it uses this
+hook to make sure the file has the current year in the copyright
+header.
 @end defvar
 
 @c Emacs 19 feature
Index: lispref/hooks.texi
===================================================================
RCS file: /cvsroot/emacs/emacs/lispref/hooks.texi,v
retrieving revision 1.14
diff -u -p -r1.14 hooks.texi
--- lispref/hooks.texi	1 Sep 2003 15:45:41 -0000	1.14
+++ lispref/hooks.texi	1 Jan 2004 12:21:08 -0000
@@ -1,6 +1,6 @@
 @c -*-texinfo-*-
 @c This is part of the GNU Emacs Lisp Reference Manual.
-@c Copyright (C) 1990, 1991, 1992, 1993, 1998 Free Software Foundation, Inc.
+@c Copyright (C) 1990, 1991, 1992, 1993, 1998, 2004 Free Software Foundation, Inc.
 @c See the file elisp.texi for copying conditions.
 @setfilename ../info/hooks
 @node Standard Hooks, Index, Standard Keymaps, Top
@@ -47,6 +47,7 @@ however, we have renamed all of those.)
 @item before-init-hook
 @item before-make-frame-hook
 @item before-revert-hook
+@item before-save-hook
 @item blink-paren-function
 @item buffer-access-fontify-functions
 @item c-mode-hook

Re: Defcustom write-file-functions and write-contents-functions?

[Eli Zaretskii]

> From: Simon Josefsson <jas@extundo.com>
> Date: Thu, 01 Jan 2004 13:22:17 +0100
> >
> > This addition should be followed by 2 documentation changes: (1) to
> > the ELisp manual, and (2) to etc/NEWS.
> 
> Yes, although I waited with that until people said it was a good idea
> to install it.  Maybe that time is now?  If so:

Thanks!

Re: Defcustom write-file-functions and write-contents-functions?

[Stefan Monnier]

> You convinced me, thanks for the feedback.  How about this patch,
> instead?

> 	* files.el (before-save-hook): Add.
> 	(basic-save-buffer): Use before-save-hook.

Please don't.  There are already plenty of hooks run at that place
and the code is hairy enough as it is.

> 	* emacs-lisp/copyright.el: Use before-save-hook instead of
> 	write-file-functions.

Better provide a copyright-auto-update-mode that will do the
hook manipulation.


        Stefan

Re: Defcustom write-file-functions and write-contents-functions?

[Luc Teirlinck]

Stefan Monnier wrote:

   Please don't.  There are already plenty of hooks run at that place
   and the code is hairy enough as it is.

It would add a line to the code, but it would not increase complexity.
Complexity is not measured in number of lines of code.

   > 	* emacs-lisp/copyright.el: Use before-save-hook instead of
   > 	write-file-functions.

   Better provide a copyright-auto-update-mode that will do the
   hook manipulation.

There might be other things users might want to have done before
saving a file and the proposed hook would very much _simplify_ this.
I realize that one always has to be extremely careful when running
something before saving _any_ file under _any_ conditions.  I
personally would not want to add, say, `delete-trailing-whitespace' to
`before-save-hook', because it would be too dangerous and would remove
wanted indentation in files that are saved while still being edited.
But certain things might be safe.  Certainly, anything that queries
the user is.  For instance, _detecting_ trailing whitespace and asking
the user is safe.

The only thing that _might_ work with the current hooks is to add
something to the front of `write-contents-functions'.  (The other
hooks in the `or' form are not guaranteed to run.)  But that solution
is not very solid, because `write-contents-functions' is not a normal
hook and something else (major modes for instance) might be playing
games with it.

Sincerely,

Luc.

Re: Defcustom write-file-functions and write-contents-functions?

[Miles Bader]

On Sun, Jan 04, 2004 at 06:14:20PM -0600, Luc Teirlinck wrote:
>    Please don't.  There are already plenty of hooks run at that place
>    and the code is hairy enough as it is.
> 
> It would add a line to the code, but it would not increase complexity.
> Complexity is not measured in number of lines of code.

It adds complexity because the number of `non local' things you have to worry
about to determine what happens when you save a file has increased.  I'd
guess that man people have been surprised by write-file-functions, but by now
know to look there if something funny is happening when they save a file.
If you add _another_ hook, then they have to repeat the process (be confused,
whoa, write-file-functions is nil, be _really_ confused, ..., eventually
realize there's now before-save-hook too, be enlightened).

That said, maybe it's worth it, maybe it's not -- maybe simply being very
careful about verbosely documenting things would be enough -- but geez, give
Stefan some credit!

-Miles
-- 
"1971 pickup truck; will trade for guns"

Re: Defcustom write-file-functions and write-contents-functions?

[Simon Josefsson]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> You convinced me, thanks for the feedback.  How about this patch,
>> instead?
>
>> 	* files.el (before-save-hook): Add.
>> 	(basic-save-buffer): Use before-save-hook.
>
> Please don't.  There are already plenty of hooks run at that place
> and the code is hairy enough as it is.

It appears as if none of the existing hooks can support the feature
that was needed here -- unconditionally invoking some code before
saving a buffer.  This seems like a rather simple and useful hook.

Abusing write-file-functions or write-contents-functions for
non-writing related code (like copyright) seems wrong.

>> 	* emacs-lisp/copyright.el: Use before-save-hook instead of
>> 	write-file-functions.
>
> Better provide a copyright-auto-update-mode that will do the
> hook manipulation.

And still use write-file-functions?  I think Luc demonstrated that
using those hooks are unreliable, so using them might not end up
invoking the copyright code if some other package uses w-f-f or w-c-f.
Anyone using ange-ftp or tramp or something to edit remote files
(which presumable hooks itself into w-f-f?) and use copyright.el too?

Re: Defcustom write-file-functions and write-contents-functions?

[Richard Stallman]

    >> 	* files.el (before-save-hook): Add.
    >> 	(basic-save-buffer): Use before-save-hook.
    >
    > Please don't.  There are already plenty of hooks run at that place
    > and the code is hairy enough as it is.

    It appears as if none of the existing hooks can support the feature
    that was needed here -- unconditionally invoking some code before
    saving a buffer.  This seems like a rather simple and useful hook.

I agree--we should add this.

Re: Defcustom write-file-functions and write-contents-functions?

[Simon Josefsson]

Richard Stallman <rms@gnu.org> writes:

>     >> 	* files.el (before-save-hook): Add.
>     >> 	(basic-save-buffer): Use before-save-hook.
>     >
>     > Please don't.  There are already plenty of hooks run at that place
>     > and the code is hairy enough as it is.
>
>     It appears as if none of the existing hooks can support the feature
>     that was needed here -- unconditionally invoking some code before
>     saving a buffer.  This seems like a rather simple and useful hook.
>
> I agree--we should add this.

I have installed it.

Re: Defcustom write-file-functions and write-contents-functions?

[Per Abrahamsen]

Luc Teirlinck <teirllm@dms.auburn.edu> writes:

> Do you really want to make those two functions customizable through
> Custom?

If not, it would probably be a good idea to add a comment to the
defvar stating why they are not declared with decustom.

Re: Defcustom write-file-functions and write-contents-functions?

[Luc Teirlinck]

Per Abrahamsen wrote:

   Luc Teirlinck <teirllm@dms.auburn.edu> writes:

   > Do you really want to make those two functions customizable through
   > Custom?

   If not, it would probably be a good idea to add a comment to the
   defvar stating why they are not declared with decustom.

As soon as it is clear whether we want the new hook `before-save-hook'
(which I personally believe would be a good idea), I could add some
comments to the documentation strings of `write-file-functions' and
`write-contents-functions' as well as to their description in
`(elisp)Saving Buffers'.  Basically, people have been trying to use
especially `write-file-functions' for things it was not intended for
and can not be used for, because it is not guaranteed to run.  If we
add `before-save-hook', we can tell people to use that customizable,
normal, unconditionally run hook instead.

Sincerely,

Luc.

Re: Defcustom write-file-functions and write-contents-functions?

[Luc Teirlinck]

Per Abrahamsen wrote:

   Luc Teirlinck <teirllm@dms.auburn.edu> writes:

   > Do you really want to make those two functions customizable through
   > Custom?

   If not, it would probably be a good idea to add a comment to the
   defvar stating why they are not declared with decustom.

I propose to add some clarifications to the docstrings and to
`(elisp)Saving Buffers'.  I could install these, if desired.

===File ~/files-diff========================================
*** files.el.~1.675.~	Mon Jan  5 12:45:52 2004
--- files.el	Tue Jan  6 19:49:22 2004
***************
*** 375,381 ****
  the visited file name with \\[set-visited-file-name], but not when you
  change the major mode.
  
! See also `write-contents-functions'.")
  (put 'write-file-functions 'permanent-local t)
  (defvaralias 'write-file-hooks 'write-file-functions)
  (make-obsolete-variable 'write-file-hooks 'write-file-functions "21.4")
--- 375,386 ----
  the visited file name with \\[set-visited-file-name], but not when you
  change the major mode.
  
! This hook is not run if any of the functions in
! `write-contents-functions' returns non-nil.  Both hooks pertain
! to how to save a buffer to file, for instance, choosing a suitable
! coding system and setting mode bits.  (See Info
! node `(elisp)Saving Buffers'.)  To perform various checks or
! updates before the buffer is saved, use `before-save-hook' .")
  (put 'write-file-functions 'permanent-local t)
  (defvaralias 'write-file-hooks 'write-file-functions)
  (make-obsolete-variable 'write-file-hooks 'write-file-functions "21.4")
***************
*** 395,401 ****
  `set-visited-file-name' does not clear this variable; but changing the
  major mode does clear it.
  
! See also `write-file-functions'.")
  (make-variable-buffer-local 'write-contents-functions)
  (defvaralias 'write-contents-hooks 'write-contents-functions)
  (make-obsolete-variable 'write-contents-hooks 'write-contents-functions "21.4")
--- 400,410 ----
  `set-visited-file-name' does not clear this variable; but changing the
  major mode does clear it.
  
! For hooks that _do_ pertain to the particular visited file, use
! `write-file-functions'.  Both this variable and
! `write-file-functions' relate to how a buffer is saved to file.
! To perform various checks or updates before the buffer is saved,
! use `before-save-hook'.")
  (make-variable-buffer-local 'write-contents-functions)
  (defvaralias 'write-contents-hooks 'write-contents-functions)
  (make-obsolete-variable 'write-contents-hooks 'write-contents-functions "21.4")
============================================================

===File ~/files.texi-diff===================================
*** files.texi.~1.60.~	Mon Jan  5 12:46:02 2004
--- files.texi	Tue Jan  6 19:45:28 2004
***************
*** 404,420 ****
  This works just like @code{write-file-functions}, but it is intended for
  hooks that pertain to the contents of the file, as opposed to hooks that
  pertain to where the file came from.  Such hooks are usually set up by
! major modes, as buffer-local bindings for this variable.
  
  This variable automatically becomes buffer-local whenever it is set;
! switching to a new major mode always resets this variable.
  @end defvar
  
  @defvar before-save-hook
! This normal hook runs before a buffer has been saved in its visited
! file.  One use of this hook is for the Copyright package; it uses this
! hook to make sure the file has the current year in the copyright
! header.
  @end defvar
  
  @c Emacs 19 feature
--- 404,424 ----
  This works just like @code{write-file-functions}, but it is intended for
  hooks that pertain to the contents of the file, as opposed to hooks that
  pertain to where the file came from.  Such hooks are usually set up by
! major modes, as buffer-local bindings for this variable.  If any of the
! functions in this hook returns non-@code{nil}, @code{write-file-functions}
! is not run.
  
  This variable automatically becomes buffer-local whenever it is set;
! switching to a new major mode always resets this variable, but
! calling @code{set-visited-file-name} does not.
  @end defvar
  
  @defvar before-save-hook
! This normal hook runs before a buffer is saved in its visited file,
! regardless of whether that is done normally or by one of the hooks
! described above.  One use of this hook is for the Copyright package;
! it uses this hook to make sure the file has the current year in the
! copyright header.
  @end defvar
  
  @c Emacs 19 feature
============================================================

Re: Defcustom write-file-functions and write-contents-functions?

[Luc Teirlinck]

Avtually, in my previous files.texi diff, I forgot to make a relevant
change: make the defvar's for `before-save-hook' and `after-save-hook'
into defopt's:

===File ~/files.texi-newdiff================================
*** files.texi.~1.60.~	Mon Jan  5 12:46:02 2004
--- files.texi	Tue Jan  6 20:50:23 2004
***************
*** 404,428 ****
  This works just like @code{write-file-functions}, but it is intended for
  hooks that pertain to the contents of the file, as opposed to hooks that
  pertain to where the file came from.  Such hooks are usually set up by
! major modes, as buffer-local bindings for this variable.
  
  This variable automatically becomes buffer-local whenever it is set;
! switching to a new major mode always resets this variable.
  @end defvar
  
! @defvar before-save-hook
! This normal hook runs before a buffer has been saved in its visited
! file.  One use of this hook is for the Copyright package; it uses this
! hook to make sure the file has the current year in the copyright
! header.
! @end defvar
  
  @c Emacs 19 feature
! @defvar after-save-hook
  This normal hook runs after a buffer has been saved in its visited file.
  One use of this hook is in Fast Lock mode; it uses this hook to save the
  highlighting information in a cache file.
! @end defvar
  
  @defvar file-precious-flag
  If this variable is non-@code{nil}, then @code{save-buffer} protects
--- 404,432 ----
  This works just like @code{write-file-functions}, but it is intended for
  hooks that pertain to the contents of the file, as opposed to hooks that
  pertain to where the file came from.  Such hooks are usually set up by
! major modes, as buffer-local bindings for this variable.  If any of the
! functions in this hook returns non-@code{nil}, @code{write-file-functions}
! is not run.
  
  This variable automatically becomes buffer-local whenever it is set;
! switching to a new major mode always resets this variable, but
! calling @code{set-visited-file-name} does not.
  @end defvar
  
! @defopt before-save-hook
! This normal hook runs before a buffer is saved in its visited file,
! regardless of whether that is done normally or by one of the hooks
! described above.  One use of this hook is for the Copyright package;
! it uses this hook to make sure the file has the current year in the
! copyright header.
! @end defopt
  
  @c Emacs 19 feature
! @defopt after-save-hook
  This normal hook runs after a buffer has been saved in its visited file.
  One use of this hook is in Fast Lock mode; it uses this hook to save the
  highlighting information in a cache file.
! @end defopt
  
  @defvar file-precious-flag
  If this variable is non-@code{nil}, then @code{save-buffer} protects
============================================================

Re: Defcustom write-file-functions and write-contents-functions?

[Richard Stallman]

Your changes are good.