all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* Proposal: move write-contents-functions higher up in basic-save-buffer
@ 2017-05-23  7:19 Eric Abrahamsen
  2017-05-23  7:25 ` Eric Abrahamsen
  2017-05-23 18:39 ` Eli Zaretskii
  0 siblings, 2 replies; 12+ messages in thread
From: Eric Abrahamsen @ 2017-05-23  7:19 UTC (permalink / raw)
  To: emacs-devel

Most special-mode buffers aren't visiting a file, and thus they miss out
on all the `do-auto-save' and `save-some-buffers' mechanisms. I'd guess
a fair number of packages that use special-mode *do* have some concept
of saving, or persisting data in some other way.

I think the `write-contents-functions' hook would be an ideal way of
solving this problem, except that the way `basic-save-buffer' is
written, it won't let you get that far without having a file name.

My proposal is to declare `write-contents-functions' as *explicitly* a
hook for buffers that don't have any file associated with them at all
(this would be in contrast to `write-file-functions'). Then we'd move it
up higher in the process: either earlier in `basic-save-buffer', or all
the way up to `save-buffer' -- that way `basic-save-buffer' could only
be for buffers that have a file.

Then `save-some-buffers' could check for the buffer-local presence of
this variable, and do the save. `do-auto-save' would behave the same.
"s" could be bound to `save-buffer' by default in special-mode.

WDYT? I think the docstring of `write-contents-functions' already
supports this interpretation, it just needs a bit of tweaking to divorce
it from buffer-file-name altogether.

Eric

PS: My original idea was to introduce a buffer-local
`save-buffer-function' variable, but I think this makes more sense.




^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Proposal: move write-contents-functions higher up in basic-save-buffer
  2017-05-23  7:19 Proposal: move write-contents-functions higher up in basic-save-buffer Eric Abrahamsen
@ 2017-05-23  7:25 ` Eric Abrahamsen
  2017-05-23 18:41   ` Eli Zaretskii
  2017-05-23 18:39 ` Eli Zaretskii
  1 sibling, 1 reply; 12+ messages in thread
From: Eric Abrahamsen @ 2017-05-23  7:25 UTC (permalink / raw)
  To: emacs-devel

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> Most special-mode buffers aren't visiting a file, and thus they miss out
> on all the `do-auto-save' and `save-some-buffers' mechanisms. I'd guess
> a fair number of packages that use special-mode *do* have some concept
> of saving, or persisting data in some other way.
>
> I think the `write-contents-functions' hook would be an ideal way of
> solving this problem, except that the way `basic-save-buffer' is
> written, it won't let you get that far without having a file name.
>
> My proposal is to declare `write-contents-functions' as *explicitly* a
> hook for buffers that don't have any file associated with them at all
> (this would be in contrast to `write-file-functions'). Then we'd move it
> up higher in the process: either earlier in `basic-save-buffer', or all
> the way up to `save-buffer' -- that way `basic-save-buffer' could only
> be for buffers that have a file.
>
> Then `save-some-buffers' could check for the buffer-local presence of
> this variable, and do the save. `do-auto-save' would behave the same.
> "s" could be bound to `save-buffer' by default in special-mode.

I forgot to say, auto-save would obviously be more difficult, since
you'd have to handle the file name and location for the auto save file.
I think it would be worth coming up with a solution, though.




^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Proposal: move write-contents-functions higher up in basic-save-buffer
  2017-05-23  7:19 Proposal: move write-contents-functions higher up in basic-save-buffer Eric Abrahamsen
  2017-05-23  7:25 ` Eric Abrahamsen
@ 2017-05-23 18:39 ` Eli Zaretskii
  2017-05-23 23:09   ` Eric Abrahamsen
  1 sibling, 1 reply; 12+ messages in thread
From: Eli Zaretskii @ 2017-05-23 18:39 UTC (permalink / raw)
  To: Eric Abrahamsen; +Cc: emacs-devel

> From: Eric Abrahamsen <eric@ericabrahamsen.net>
> Date: Tue, 23 May 2017 15:19:11 +0800
> 
> Most special-mode buffers aren't visiting a file, and thus they miss out
> on all the `do-auto-save' and `save-some-buffers' mechanisms. I'd guess
> a fair number of packages that use special-mode *do* have some concept
> of saving, or persisting data in some other way.
> 
> I think the `write-contents-functions' hook would be an ideal way of
> solving this problem, except that the way `basic-save-buffer' is
> written, it won't let you get that far without having a file name.
> 
> My proposal is to declare `write-contents-functions' as *explicitly* a
> hook for buffers that don't have any file associated with them at all
> (this would be in contrast to `write-file-functions'). Then we'd move it
> up higher in the process: either earlier in `basic-save-buffer', or all
> the way up to `save-buffer' -- that way `basic-save-buffer' could only
> be for buffers that have a file.

Did you investigate the alternative -- teach basic-save-buffer to save
buffers that don't visit files?  If that's possible, it should be
easier.



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Proposal: move write-contents-functions higher up in basic-save-buffer
  2017-05-23  7:25 ` Eric Abrahamsen
@ 2017-05-23 18:41   ` Eli Zaretskii
  0 siblings, 0 replies; 12+ messages in thread
From: Eli Zaretskii @ 2017-05-23 18:41 UTC (permalink / raw)
  To: Eric Abrahamsen; +Cc: emacs-devel

> From: Eric Abrahamsen <eric@ericabrahamsen.net>
> Date: Tue, 23 May 2017 15:25:01 +0800
> 
> I forgot to say, auto-save would obviously be more difficult, since
> you'd have to handle the file name and location for the auto save file.

Perhaps for such buffers it would make sense to make
auto-save-visited-mode the default.  Then these problems will be
automatically taken care of, if you teach basic-save-buffer to save
such buffers, per my proposal.



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Proposal: move write-contents-functions higher up in basic-save-buffer
  2017-05-23 18:39 ` Eli Zaretskii
@ 2017-05-23 23:09   ` Eric Abrahamsen
  2017-05-24  2:38     ` Eli Zaretskii
                       ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Eric Abrahamsen @ 2017-05-23 23:09 UTC (permalink / raw)
  To: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Eric Abrahamsen <eric@ericabrahamsen.net>
>> Date: Tue, 23 May 2017 15:19:11 +0800
>> 
>> Most special-mode buffers aren't visiting a file, and thus they miss out
>> on all the `do-auto-save' and `save-some-buffers' mechanisms. I'd guess
>> a fair number of packages that use special-mode *do* have some concept
>> of saving, or persisting data in some other way.
>> 
>> I think the `write-contents-functions' hook would be an ideal way of
>> solving this problem, except that the way `basic-save-buffer' is
>> written, it won't let you get that far without having a file name.
>> 
>> My proposal is to declare `write-contents-functions' as *explicitly* a
>> hook for buffers that don't have any file associated with them at all
>> (this would be in contrast to `write-file-functions'). Then we'd move it
>> up higher in the process: either earlier in `basic-save-buffer', or all
>> the way up to `save-buffer' -- that way `basic-save-buffer' could only
>> be for buffers that have a file.
>
> Did you investigate the alternative -- teach basic-save-buffer to save
> buffers that don't visit files?  If that's possible, it should be
> easier.

I thought that's what I was doing! If a buffer isn't visiting a file,
there's essentially no way to guess what "saving it" would mean. The
mode that created the buffer would need to provide a function that does
the saving. Then basic-save-buffer would need to be taught to call that
function, instead of insisting that the buffer have a file.

My original thought was to have a new buffer-local variable,
save-buffer-function, that points to this function. Then it occurred to
me that write-contents-functions seems like a good place to do this. Now
I'm not sure.

Eric




^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Proposal: move write-contents-functions higher up in basic-save-buffer
  2017-05-23 23:09   ` Eric Abrahamsen
@ 2017-05-24  2:38     ` Eli Zaretskii
  2017-05-24  4:55       ` Eric Abrahamsen
  2017-05-24 14:12     ` Richard Stallman
  2017-05-28 10:12     ` Eric Abrahamsen
  2 siblings, 1 reply; 12+ messages in thread
From: Eli Zaretskii @ 2017-05-24  2:38 UTC (permalink / raw)
  To: Eric Abrahamsen; +Cc: emacs-devel

> From: Eric Abrahamsen <eric@ericabrahamsen.net>
> Date: Wed, 24 May 2017 07:09:07 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: Eric Abrahamsen <eric@ericabrahamsen.net>
> >> Date: Tue, 23 May 2017 15:19:11 +0800
> >> 
> >> Most special-mode buffers aren't visiting a file, and thus they miss out
> >> on all the `do-auto-save' and `save-some-buffers' mechanisms. I'd guess
> >> a fair number of packages that use special-mode *do* have some concept
> >> of saving, or persisting data in some other way.
> >> 
> >> I think the `write-contents-functions' hook would be an ideal way of
> >> solving this problem, except that the way `basic-save-buffer' is
> >> written, it won't let you get that far without having a file name.
> >> 
> >> My proposal is to declare `write-contents-functions' as *explicitly* a
> >> hook for buffers that don't have any file associated with them at all
> >> (this would be in contrast to `write-file-functions'). Then we'd move it
> >> up higher in the process: either earlier in `basic-save-buffer', or all
> >> the way up to `save-buffer' -- that way `basic-save-buffer' could only
> >> be for buffers that have a file.
> >
> > Did you investigate the alternative -- teach basic-save-buffer to save
> > buffers that don't visit files?  If that's possible, it should be
> > easier.
> 
> I thought that's what I was doing!

I was referring specifically to this party of your description:

> I think the `write-contents-functions' hook would be an ideal way of
> solving this problem, except that the way `basic-save-buffer' is
> written, it won't let you get that far without having a file name.

My thinking was that by somehow overcoming this obstacle, you can
allow users to easily use write-contents-functions as they need.

Does this make sense?  If not, can you tell what is the difficulty in
this regard?



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Proposal: move write-contents-functions higher up in basic-save-buffer
  2017-05-24  2:38     ` Eli Zaretskii
@ 2017-05-24  4:55       ` Eric Abrahamsen
  2017-05-24 12:29         ` Stefan Monnier
  2017-05-24 17:46         ` Eli Zaretskii
  0 siblings, 2 replies; 12+ messages in thread
From: Eric Abrahamsen @ 2017-05-24  4:55 UTC (permalink / raw)
  To: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Eric Abrahamsen <eric@ericabrahamsen.net>
>> Date: Wed, 24 May 2017 07:09:07 +0800
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> >> From: Eric Abrahamsen <eric@ericabrahamsen.net>
>> >> Date: Tue, 23 May 2017 15:19:11 +0800
>> >> 
>> >> Most special-mode buffers aren't visiting a file, and thus they miss out
>> >> on all the `do-auto-save' and `save-some-buffers' mechanisms. I'd guess
>> >> a fair number of packages that use special-mode *do* have some concept
>> >> of saving, or persisting data in some other way.
>> >> 
>> >> I think the `write-contents-functions' hook would be an ideal way of
>> >> solving this problem, except that the way `basic-save-buffer' is
>> >> written, it won't let you get that far without having a file name.
>> >> 
>> >> My proposal is to declare `write-contents-functions' as *explicitly* a
>> >> hook for buffers that don't have any file associated with them at all
>> >> (this would be in contrast to `write-file-functions'). Then we'd move it
>> >> up higher in the process: either earlier in `basic-save-buffer', or all
>> >> the way up to `save-buffer' -- that way `basic-save-buffer' could only
>> >> be for buffers that have a file.
>> >
>> > Did you investigate the alternative -- teach basic-save-buffer to save
>> > buffers that don't visit files?  If that's possible, it should be
>> > easier.
>> 
>> I thought that's what I was doing!
>
> I was referring specifically to this party of your description:
>
>> I think the `write-contents-functions' hook would be an ideal way of
>> solving this problem, except that the way `basic-save-buffer' is
>> written, it won't let you get that far without having a file name.
>
> My thinking was that by somehow overcoming this obstacle, you can
> allow users to easily use write-contents-functions as they need.
>
> Does this make sense?  If not, can you tell what is the difficulty in
> this regard?

I probably just did a poor job writing the initial message. That's what
I was proposing to begin with: to jiggle `basic-save-buffer' (and I
think also `save-some-buffers') so that the running of
`write-contents-functions' comes earlier in the function, or is
otherwise in its own branch that doesn't require a `buffer-file-name'. I
think it would be a fairly unintrusive change, it would just require a
bit of thought. I can try to produce a patch, if this is acceptable in
principle.

Eric




^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Proposal: move write-contents-functions higher up in basic-save-buffer
  2017-05-24  4:55       ` Eric Abrahamsen
@ 2017-05-24 12:29         ` Stefan Monnier
  2017-05-25  7:42           ` Eric Abrahamsen
  2017-05-24 17:46         ` Eli Zaretskii
  1 sibling, 1 reply; 12+ messages in thread
From: Stefan Monnier @ 2017-05-24 12:29 UTC (permalink / raw)
  To: emacs-devel

> I probably just did a poor job writing the initial message. That's what
> I was proposing to begin with: to jiggle `basic-save-buffer' (and I
> think also `save-some-buffers') so that the running of
> `write-contents-functions' comes earlier in the function, or is
> otherwise in its own branch that doesn't require a `buffer-file-name'. I
> think it would be a fairly unintrusive change, it would just require a
> bit of thought. I can try to produce a patch, if this is acceptable in
> principle.

I haven't looked at basic-save-buffer recently, but in the worst case we
could keep the current code and add a

    (if (null buffer-file-name)
        (run-hook-... 'write-contents-functions)
      ...)

but admittedly, it's better if we can move the single call to
write-contents-functions so it's shared by the file and the
non-file cases.


        Stefan




^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Proposal: move write-contents-functions higher up in basic-save-buffer
  2017-05-23 23:09   ` Eric Abrahamsen
  2017-05-24  2:38     ` Eli Zaretskii
@ 2017-05-24 14:12     ` Richard Stallman
  2017-05-28 10:12     ` Eric Abrahamsen
  2 siblings, 0 replies; 12+ messages in thread
From: Richard Stallman @ 2017-05-24 14:12 UTC (permalink / raw)
  To: Eric Abrahamsen; +Cc: emacs-devel

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > I thought that's what I was doing! If a buffer isn't visiting a file,
  > there's essentially no way to guess what "saving it" would mean.

That's true, as a generality.  The only way that "saving" a non-file
buffer could be meaningful is if Emacs is told precisely what saving
should mean for that buffer.

								     The
  > mode that created the buffer would need to provide a function that does
  > the saving. Then basic-save-buffer would need to be taught to call that
  > function, instead of insisting that the buffer have a file.

  > Then it occurred to
  > me that write-contents-functions seems like a good place to do this. Now
  > I'm not sure.

Please try using write-contents-functions for this, and you'll see if it
does the job.  If not, you'll see what more change is needed.

-- 
Dr Richard Stallman
President, Free Software Foundation (gnu.org, fsf.org)
Internet Hall-of-Famer (internethalloffame.org)
Skype: No way! See stallman.org/skype.html.




^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Proposal: move write-contents-functions higher up in basic-save-buffer
  2017-05-24  4:55       ` Eric Abrahamsen
  2017-05-24 12:29         ` Stefan Monnier
@ 2017-05-24 17:46         ` Eli Zaretskii
  1 sibling, 0 replies; 12+ messages in thread
From: Eli Zaretskii @ 2017-05-24 17:46 UTC (permalink / raw)
  To: Eric Abrahamsen; +Cc: emacs-devel

> From: Eric Abrahamsen <eric@ericabrahamsen.net>
> Date: Wed, 24 May 2017 12:55:53 +0800
> 
> > My thinking was that by somehow overcoming this obstacle, you can
> > allow users to easily use write-contents-functions as they need.
> >
> > Does this make sense?  If not, can you tell what is the difficulty in
> > this regard?
> 
> I probably just did a poor job writing the initial message. That's what
> I was proposing to begin with: to jiggle `basic-save-buffer' (and I
> think also `save-some-buffers') so that the running of
> `write-contents-functions' comes earlier in the function, or is
> otherwise in its own branch that doesn't require a `buffer-file-name'.

Then I guess we are in violent agreement.

> I can try to produce a patch, if this is acceptable in principle.

Yes, please.

Thanks.



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Proposal: move write-contents-functions higher up in basic-save-buffer
  2017-05-24 12:29         ` Stefan Monnier
@ 2017-05-25  7:42           ` Eric Abrahamsen
  0 siblings, 0 replies; 12+ messages in thread
From: Eric Abrahamsen @ 2017-05-25  7:42 UTC (permalink / raw)
  To: emacs-devel

[-- Attachment #1: Type: text/plain, Size: 1261 bytes --]

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

> I haven't looked at basic-save-buffer recently, but in the worst case we
> could keep the current code and add a
>
>     (if (null buffer-file-name)
>         (run-hook-... 'write-contents-functions)
>       ...)
>
> but admittedly, it's better if we can move the single call to
> write-contents-functions so it's shared by the file and the
> non-file cases.

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Eric Abrahamsen <eric@ericabrahamsen.net>
>> Date: Wed, 24 May 2017 12:55:53 +0800

>> I can try to produce a patch, if this is acceptable in principle.
>
> Yes, please.

Richard Stallman <rms@gnu.org> writes:

> Please try using write-contents-functions for this, and you'll see if it
> does the job.  If not, you'll see what more change is needed.

Okay, here's a first stab at it. I think it should work correctly: all
the short-circuit hooks get a chance to run in all cases, but the
function only insists on the presence of a file if
`write-contents-functions' are not present, or if they fail with a nil
value. I'd like to specify in the docs that those functions should fail
with an error.

If this looks okay I'll spend a bit more time testing it, then make
docstring and manual edits.

Eric


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-First-whack-at-write-contents-functions-for-non-file.patch --]
[-- Type: text/x-diff, Size: 7052 bytes --]

From af4f811439785113fe2be71f499006776958755b Mon Sep 17 00:00:00 2001
From: Eric Abrahamsen <eric@ericabrahamsen.net>
Date: Thu, 25 May 2017 15:28:19 +0800
Subject: [PATCH] First whack at write-contents-functions for non-file buffers

---
 lisp/files.el | 106 +++++++++++++++++++++++++++++++---------------------------
 1 file changed, 56 insertions(+), 50 deletions(-)

diff --git a/lisp/files.el b/lisp/files.el
index 8ac1993754..c074fa7995 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -4943,29 +4943,14 @@ basic-save-buffer
     (if (buffer-base-buffer)
 	(set-buffer (buffer-base-buffer)))
     (if (or (buffer-modified-p)
-	    ;; handle the case when no modification has been made but
-	    ;; the file disappeared since visited
+	    ;; Handle the case when no modification has been made but
+	    ;; the file disappeared since visited.
 	    (and buffer-file-name
 		 (not (file-exists-p buffer-file-name))))
 	(let ((recent-save (recent-auto-save-p))
 	      setmodes)
-          ;; If buffer has no file name, ask user for one.
-	  (or buffer-file-name
-              (let ((filename
-                     (expand-file-name
-                      (read-file-name "File to save in: "
-                                      nil (expand-file-name (buffer-name))))))
-                (if (file-exists-p filename)
-                    (if (file-directory-p filename)
-                        ;; Signal an error if the user specified the name of an
-                        ;; existing directory.
-                        (error "%s is a directory" filename)
-                      (unless (y-or-n-p (format-message
-                                         "File `%s' exists; overwrite? "
-                                         filename))
-                        (error "Canceled"))))
-                (set-visited-file-name filename)))
-	  (or (verify-visited-file-modtime (current-buffer))
+	  (or (null buffer-file-name)
+              (verify-visited-file-modtime (current-buffer))
 	      (not (file-exists-p buffer-file-name))
 	      (yes-or-no-p
 	       (format
@@ -4977,6 +4962,7 @@ basic-save-buffer
 	    (save-excursion
 	      (and (> (point-max) (point-min))
 		   (not find-file-literally)
+                   (null buffer-read-only)
 		   (/= (char-after (1- (point-max))) ?\n)
 		   (not (and (eq selective-display t)
 			     (= (char-after (1- (point-max))) ?\r)))
@@ -4989,41 +4975,60 @@ basic-save-buffer
 		   (save-excursion
 		     (goto-char (point-max))
 		     (insert ?\n))))
-	    ;; Support VC version backups.
-	    (vc-before-save)
 	    ;; Don't let errors prevent saving the buffer.
 	    (with-demoted-errors (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)
-		;; If a hook returned t, file is already "written".
-		;; Otherwise, write it the usual way now.
-		(let ((dir (file-name-directory
-			    (expand-file-name buffer-file-name))))
-		  (unless (file-exists-p dir)
-		    (if (y-or-n-p
-			 (format-message
-                          "Directory `%s' does not exist; create? " dir))
-			(make-directory dir t)
-		      (error "Canceled")))
-		  (setq setmodes (basic-save-buffer-1))))
+            ;; Give `write-contents-functions' a chance to
+            ;; short-circuit the whole process.
+	    (unless (run-hook-with-args-until-success 'write-contents-functions)
+              ;; If buffer has no file name, ask user for one.
+              (or buffer-file-name
+                  (let ((filename
+                         (expand-file-name
+                          (read-file-name "File to save in: "
+                                          nil (expand-file-name (buffer-name))))))
+                    (if (file-exists-p filename)
+                        (if (file-directory-p filename)
+                            ;; Signal an error if the user specified the name of an
+                            ;; existing directory.
+                            (error "%s is a directory" filename)
+                          (unless (y-or-n-p (format-message
+                                             "File `%s' exists; overwrite? "
+                                             filename))
+                            (error "Canceled"))))
+                    (set-visited-file-name filename)))
+              ;; Support VC version backups.
+	      (vc-before-save)
+	      (or (run-hook-with-args-until-success 'local-write-file-hooks)
+	          (run-hook-with-args-until-success 'write-file-functions)
+	          ;; If a hook returned t, file is already "written".
+	          ;; Otherwise, write it the usual way now.
+	          (let ((dir (file-name-directory
+			      (expand-file-name buffer-file-name))))
+		    (unless (file-exists-p dir)
+		      (if (y-or-n-p
+		           (format-message
+                            "Directory `%s' does not exist; create? " dir))
+		          (make-directory dir t)
+		        (error "Canceled")))
+		    (setq setmodes (basic-save-buffer-1)))))
 	    ;; Now we have saved the current buffer.  Let's make sure
 	    ;; that buffer-file-coding-system is fixed to what
 	    ;; actually used for saving by binding it locally.
-	    (if save-buffer-coding-system
-		(setq save-buffer-coding-system last-coding-system-used)
-	      (setq buffer-file-coding-system last-coding-system-used))
-	    (setq buffer-file-number
-		  (nthcdr 10 (file-attributes buffer-file-name)))
-	    (if setmodes
-		(condition-case ()
-		    (progn
-		      (unless
-			  (with-demoted-errors
-			    (set-file-modes buffer-file-name (car setmodes)))
-			(set-file-extended-attributes buffer-file-name
-						      (nth 1 setmodes))))
-		  (error nil))))
+            (when buffer-file-name
+	      (if save-buffer-coding-system
+		  (setq save-buffer-coding-system last-coding-system-used)
+	        (setq buffer-file-coding-system last-coding-system-used))
+	      (setq buffer-file-number
+		    (nthcdr 10 (file-attributes buffer-file-name)))
+	      (if setmodes
+		  (condition-case ()
+		      (progn
+		        (unless
+			    (with-demoted-errors
+			        (set-file-modes buffer-file-name (car setmodes)))
+			  (set-file-extended-attributes buffer-file-name
+						        (nth 1 setmodes))))
+		    (error nil)))))
 	  ;; If the auto-save file was recent before this command,
 	  ;; delete it now.
 	  (delete-auto-save-file-if-necessary recent-save)
@@ -5255,7 +5260,8 @@ save-some-buffers
                      (and pred
                           (progn
                             (set-buffer buffer)
-                            (and buffer-offer-save (> (buffer-size) 0)))))
+                            (and buffer-offer-save (> (buffer-size) 0))))
+                     write-contents-functions)
                     (or (not (functionp pred))
                         (with-current-buffer buffer (funcall pred)))
                     (if arg
-- 
2.13.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: Proposal: move write-contents-functions higher up in basic-save-buffer
  2017-05-23 23:09   ` Eric Abrahamsen
  2017-05-24  2:38     ` Eli Zaretskii
  2017-05-24 14:12     ` Richard Stallman
@ 2017-05-28 10:12     ` Eric Abrahamsen
  2 siblings, 0 replies; 12+ messages in thread
From: Eric Abrahamsen @ 2017-05-28 10:12 UTC (permalink / raw)
  To: emacs-devel

[-- Attachment #1: Type: text/plain, Size: 2231 bytes --]

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> From: Eric Abrahamsen <eric@ericabrahamsen.net>
>>> Date: Tue, 23 May 2017 15:19:11 +0800
>>>
>>> Most special-mode buffers aren't visiting a file, and thus they miss out
>>> on all the `do-auto-save' and `save-some-buffers' mechanisms. I'd guess
>>> a fair number of packages that use special-mode *do* have some concept
>>> of saving, or persisting data in some other way.
>>>
>>> I think the `write-contents-functions' hook would be an ideal way of
>>> solving this problem, except that the way `basic-save-buffer' is
>>> written, it won't let you get that far without having a file name.
>>>
>>> My proposal is to declare `write-contents-functions' as *explicitly* a
>>> hook for buffers that don't have any file associated with them at all
>>> (this would be in contrast to `write-file-functions'). Then we'd move it
>>> up higher in the process: either earlier in `basic-save-buffer', or all
>>> the way up to `save-buffer' -- that way `basic-save-buffer' could only
>>> be for buffers that have a file.

Okay, I've poked at this in all the ways I can think of, and it seems to
work okay. Basic recap:

The goal is to re-interpret `write-contents-functions' as a mechanism
for allowing buffers that are not visiting a file to specify a custom
save mechanism. The original idea was to let special-mode buffers
install their own save routines, which would run on `save-buffer', and
also as a part of the `save-some-buffers' routine.

If the buffer-local value of `write-contents-functions' is non-nil for
buffer BUF, then `save-some-buffers' will accept BUF as a potentially
saveable buffer.

`basic-save-buffer' has been rearranged so that the
`write-contents-functions' hook is run a little earlier on. Only if the
functions in that hook fail will `basic-save-buffer' go on to prompt the
user for a file to save the buffer in.

I'm leaving `do-auto-save' as a problem for another day.

I've done a proper commit, with manual edits and everything. I'm a bit
leery of just committing this, as it theoretically touches every buffer
in an Emacs session. If anyone wants to take a hard stare at it, that
would be very welcome.

Eric


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Allow-write-contents-functions-to-short-circuit-buff.patch --]
[-- Type: text/x-diff, Size: 11183 bytes --]

From 7b5f18648e3d4b2aa9a5af536a624d6518d8fdd7 Mon Sep 17 00:00:00 2001
From: Eric Abrahamsen <eric@ericabrahamsen.net>
Date: Thu, 25 May 2017 15:28:19 +0800
Subject: [PATCH] Allow write-contents-functions to short-circuit buffer saving

* lisp/files.el (basic-save-buffer): If write-contents-functions is
  non-nil, give the functions in that hook a chance to save buffer
  contents before checking if buffer is visiting a file.
  (save-some-buffers): If write-contents-functions is non nil,
  consider the buffer eligible for a save prompt.
* doc/lispref/files.texi (Saving Buffers): Mention new behavior, note
  that special-mode buffers can use this to "save" themselves.
---
 doc/lispref/files.texi |  18 +++++--
 lisp/files.el          | 136 +++++++++++++++++++++++++++----------------------
 2 files changed, 87 insertions(+), 67 deletions(-)

diff --git a/doc/lispref/files.texi b/doc/lispref/files.texi
index 2b692dbf68..a6ee0cc69c 100644
--- a/doc/lispref/files.texi
+++ b/doc/lispref/files.texi
@@ -457,15 +457,23 @@ Saving Buffers
 @defvar write-contents-functions
 This works just like @code{write-file-functions}, but it is intended
 for hooks that pertain to the buffer's contents, not to the particular
-visited file or its location.  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, but calling
-@code{set-visited-file-name} does not.
+visited file or its location, and can be used to create arbitrary save
+processes for buffers that aren't visiting files at all.  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,
+but calling @code{set-visited-file-name} does not.
 
 If any of the functions in this hook returns non-@code{nil}, the file
 is considered already written and the rest are not called and neither
 are the functions in @code{write-file-functions}.
+
+When using this hook to save buffers that are not visiting files (for
+instance, special-mode buffers), keep in mind that, if the function
+fails to save correctly and returns a @code{nil} value,
+@code{save-buffer} will go on to prompt the user for a file to save
+the buffer in.  If this is undesirable, consider having the function
+fail by raising an error.
 @end defvar
 
 @defopt before-save-hook
diff --git a/lisp/files.el b/lisp/files.el
index 8ac1993754..1f88f86b76 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -514,10 +514,12 @@ 'write-contents-hooks
     'write-contents-functions "22.1")
 (defvar write-contents-functions nil
   "List of functions to be called before writing out a buffer to a file.
-Only used by `save-buffer'.
-If one of them returns non-nil, the file is considered already written
-and the rest are not called and neither are the functions in
-`write-file-functions'.
+
+Only used by `save-buffer'.  If one of them returns non-nil, the
+file is considered already written and the rest are not called
+and neither are the functions in `write-file-functions'.  This
+hook can thus be used to create save behavior for buffers that
+are not visiting a file at all.
 
 This variable is meant to be used for hooks that pertain to the
 buffer's contents, not to the particular visited file; thus,
@@ -4932,9 +4934,12 @@ save-buffer-coding-system
 
 (defun basic-save-buffer (&optional called-interactively)
   "Save the current buffer in its visited file, if it has been modified.
-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 in the usual way.
+
+The hooks `write-contents-functions', `local-write-file-hooks'
+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 in
+the usual way.
+
 Before and after saving the buffer, this function runs
 `before-save-hook' and `after-save-hook', respectively."
   (interactive '(called-interactively))
@@ -4943,29 +4948,14 @@ basic-save-buffer
     (if (buffer-base-buffer)
 	(set-buffer (buffer-base-buffer)))
     (if (or (buffer-modified-p)
-	    ;; handle the case when no modification has been made but
-	    ;; the file disappeared since visited
+	    ;; Handle the case when no modification has been made but
+	    ;; the file disappeared since visited.
 	    (and buffer-file-name
 		 (not (file-exists-p buffer-file-name))))
 	(let ((recent-save (recent-auto-save-p))
 	      setmodes)
-          ;; If buffer has no file name, ask user for one.
-	  (or buffer-file-name
-              (let ((filename
-                     (expand-file-name
-                      (read-file-name "File to save in: "
-                                      nil (expand-file-name (buffer-name))))))
-                (if (file-exists-p filename)
-                    (if (file-directory-p filename)
-                        ;; Signal an error if the user specified the name of an
-                        ;; existing directory.
-                        (error "%s is a directory" filename)
-                      (unless (y-or-n-p (format-message
-                                         "File `%s' exists; overwrite? "
-                                         filename))
-                        (error "Canceled"))))
-                (set-visited-file-name filename)))
-	  (or (verify-visited-file-modtime (current-buffer))
+	  (or (null buffer-file-name)
+              (verify-visited-file-modtime (current-buffer))
 	      (not (file-exists-p buffer-file-name))
 	      (yes-or-no-p
 	       (format
@@ -4977,6 +4967,7 @@ basic-save-buffer
 	    (save-excursion
 	      (and (> (point-max) (point-min))
 		   (not find-file-literally)
+                   (null buffer-read-only)
 		   (/= (char-after (1- (point-max))) ?\n)
 		   (not (and (eq selective-display t)
 			     (= (char-after (1- (point-max))) ?\r)))
@@ -4989,46 +4980,65 @@ basic-save-buffer
 		   (save-excursion
 		     (goto-char (point-max))
 		     (insert ?\n))))
-	    ;; Support VC version backups.
-	    (vc-before-save)
 	    ;; Don't let errors prevent saving the buffer.
 	    (with-demoted-errors (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)
-		;; If a hook returned t, file is already "written".
-		;; Otherwise, write it the usual way now.
-		(let ((dir (file-name-directory
-			    (expand-file-name buffer-file-name))))
-		  (unless (file-exists-p dir)
-		    (if (y-or-n-p
-			 (format-message
-                          "Directory `%s' does not exist; create? " dir))
-			(make-directory dir t)
-		      (error "Canceled")))
-		  (setq setmodes (basic-save-buffer-1))))
+            ;; Give `write-contents-functions' a chance to
+            ;; short-circuit the whole process.
+	    (unless (run-hook-with-args-until-success 'write-contents-functions)
+              ;; If buffer has no file name, ask user for one.
+              (or buffer-file-name
+                  (let ((filename
+                         (expand-file-name
+                          (read-file-name "File to save in: "
+                                          nil (expand-file-name (buffer-name))))))
+                    (if (file-exists-p filename)
+                        (if (file-directory-p filename)
+                            ;; Signal an error if the user specified the name of an
+                            ;; existing directory.
+                            (error "%s is a directory" filename)
+                          (unless (y-or-n-p (format-message
+                                             "File `%s' exists; overwrite? "
+                                             filename))
+                            (error "Canceled"))))
+                    (set-visited-file-name filename)))
+              ;; Support VC version backups.
+	      (vc-before-save)
+	      (or (run-hook-with-args-until-success 'local-write-file-hooks)
+	          (run-hook-with-args-until-success 'write-file-functions)
+	          ;; If a hook returned t, file is already "written".
+	          ;; Otherwise, write it the usual way now.
+	          (let ((dir (file-name-directory
+			      (expand-file-name buffer-file-name))))
+		    (unless (file-exists-p dir)
+		      (if (y-or-n-p
+		           (format-message
+                            "Directory `%s' does not exist; create? " dir))
+		          (make-directory dir t)
+		        (error "Canceled")))
+		    (setq setmodes (basic-save-buffer-1)))))
 	    ;; Now we have saved the current buffer.  Let's make sure
 	    ;; that buffer-file-coding-system is fixed to what
 	    ;; actually used for saving by binding it locally.
-	    (if save-buffer-coding-system
-		(setq save-buffer-coding-system last-coding-system-used)
-	      (setq buffer-file-coding-system last-coding-system-used))
-	    (setq buffer-file-number
-		  (nthcdr 10 (file-attributes buffer-file-name)))
-	    (if setmodes
-		(condition-case ()
-		    (progn
-		      (unless
-			  (with-demoted-errors
-			    (set-file-modes buffer-file-name (car setmodes)))
-			(set-file-extended-attributes buffer-file-name
-						      (nth 1 setmodes))))
-		  (error nil))))
-	  ;; If the auto-save file was recent before this command,
-	  ;; delete it now.
-	  (delete-auto-save-file-if-necessary recent-save)
-	  ;; Support VC `implicit' locking.
-	  (vc-after-save)
+            (when buffer-file-name
+	      (if save-buffer-coding-system
+		  (setq save-buffer-coding-system last-coding-system-used)
+	        (setq buffer-file-coding-system last-coding-system-used))
+	      (setq buffer-file-number
+		    (nthcdr 10 (file-attributes buffer-file-name)))
+	      (if setmodes
+		  (condition-case ()
+		      (progn
+		        (unless
+			    (with-demoted-errors
+			        (set-file-modes buffer-file-name (car setmodes)))
+			  (set-file-extended-attributes buffer-file-name
+						        (nth 1 setmodes))))
+		    (error nil)))
+              ;; Support VC `implicit' locking.
+	      (vc-after-save))
+            ;; If the auto-save file was recent before this command,
+	    ;; delete it now.
+	    (delete-auto-save-file-if-necessary recent-save))
 	  (run-hooks 'after-save-hook))
       (or noninteractive
           (not called-interactively)
@@ -5255,7 +5265,9 @@ save-some-buffers
                      (and pred
                           (progn
                             (set-buffer buffer)
-                            (and buffer-offer-save (> (buffer-size) 0)))))
+                            (and buffer-offer-save (> (buffer-size) 0))))
+                     (buffer-local-value
+                      'write-contents-functions buffer))
                     (or (not (functionp pred))
                         (with-current-buffer buffer (funcall pred)))
                     (if arg
-- 
2.13.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2017-05-28 10:12 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-05-23  7:19 Proposal: move write-contents-functions higher up in basic-save-buffer Eric Abrahamsen
2017-05-23  7:25 ` Eric Abrahamsen
2017-05-23 18:41   ` Eli Zaretskii
2017-05-23 18:39 ` Eli Zaretskii
2017-05-23 23:09   ` Eric Abrahamsen
2017-05-24  2:38     ` Eli Zaretskii
2017-05-24  4:55       ` Eric Abrahamsen
2017-05-24 12:29         ` Stefan Monnier
2017-05-25  7:42           ` Eric Abrahamsen
2017-05-24 17:46         ` Eli Zaretskii
2017-05-24 14:12     ` Richard Stallman
2017-05-28 10:12     ` Eric Abrahamsen

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.