4 minor suggestions for files.el

4 minor suggestions for files.el

[Stefan Monnier]

1 - Don't allow viewing non-existent files.  The interactive spec already
    ensures that's the case, but it doesn't work when the function
    is called from dired or somesuch (the file might have existed
    when the dired buffer was created).

@@ -928,6 +928,7 @@
 Like \\[find-file] but marks buffer as read-only.
 Use \\[toggle-read-only] to permit editing."
   (interactive (find-file-read-args "Find file read-only: " t))
+  (unless (file-exists-p filename) (error "%s does not exist" filename))
   (find-file filename wildcards)
   (toggle-read-only 1)
   (current-buffer))
@@ -937,6 +938,7 @@
 Like \\[find-file-other-window] but marks buffer as read-only.
 Use \\[toggle-read-only] to permit editing."
   (interactive (find-file-read-args "Find file read-only other window: " t))
+  (unless (file-exists-p filename) (error "%s does not exist" filename))
   (find-file-other-window filename wildcards)
   (toggle-read-only 1)
   (current-buffer))
@@ -946,6 +948,7 @@
 Like \\[find-file-other-frame] but marks buffer as read-only.
 Use \\[toggle-read-only] to permit editing."
   (interactive (find-file-read-args "Find file read-only other frame: " t))
+  (unless (file-exists-p filename) (error "%s does not exist" filename))
   (find-file-other-frame filename wildcards)
   (toggle-read-only 1)
   (current-buffer))


2 - Prompt the user when trying to open a very large file.  My Emacs crawls
    to a near halt when I try to open a 16MB file, so if I ever try to
    open such a file, I'd rather be warned.

@@ -1163,6 +1166,9 @@
   :version "21.1"
   :type 'boolean)
 
+(defcustom large-file-warning-threshold 10000000
+  "Maximum size of file above which a confirmation is requested.")
+
 (defun find-file-noselect (filename &optional nowarn rawfile wildcards)
   "Read file FILENAME into a buffer and return the buffer.
 If a buffer exists visiting FILENAME, return that one, but
@@ -1198,7 +1204,8 @@
 	    (mapcar #'find-file-noselect files)))
       (let* ((buf (get-file-buffer filename))
 	     (truename (abbreviate-file-name (file-truename filename)))
-	     (number (nthcdr 10 (file-attributes truename)))
+	     (attributes (file-attributes truename))
+	     (number (nthcdr 10 attributes))
 	     ;; Find any buffer for a file which has same truename.
 	     (other (and (not buf) (find-buffer-visiting filename))))
 	;; Let user know if there is a buffer with the same truename.
@@ -1212,6 +1219,17 @@
 	      ;; Optionally also find that buffer.
 	      (if (or find-file-existing-other-name find-file-visit-truename)
 		  (setq buf other))))
+	;; Check to see if the file looks uncommonly large.
+	(when (and large-file-warning-threshold (nth 7 attributes)
+		   ;; Don't ask again if we already have the file or
+		   ;; if we're asked to be quiet.
+		   (not (or buf nowarn))
+		   (> (nth 7 attributes) large-file-warning-threshold)
+		   (not (y-or-n-p
+			 (format "File %s is large (%sMB), really open? "
+				 (file-name-nondirectory filename)
+				   (/ (nth 7 attributes) 1048576)))))
+	  (error "Aborted"))
 	(if buf
 	    ;; We are using an existing buffer.
 	    (progn


3 - Don't catch errors to turn them into messages when debug-on-error
    is set to t.  I wish I could solve it more generically.

@@ -1521,6 +1539,17 @@
       (view-mode-enter))
     (run-hooks 'find-file-hook)))
 
+(defmacro with-errors-caught (format &rest body)
+  "Eval BODY and turn any error into a FORMAT message.
+FORMAT can have a %s escape which will be replaced with the actual error.
+If `debug-on-error' is set, errors are not caught, so that you can
+debug them."
+  `(if debug-on-error
+       (progn . ,body)
+     (condition-case err
+	 (progn . ,body)
+       (error (message ,format (prin1-to-string err))))))
+
 (defun normal-mode (&optional find-file)
   "Choose the major mode for this buffer automatically.
 Also sets up any specified local variables of the file.
@@ -1538,16 +1567,11 @@
 in that case, this function acts as if `enable-local-variables' were t."
   (interactive)
   (or find-file (funcall (or default-major-mode 'fundamental-mode)))
-  (condition-case err
-      (set-auto-mode)
-    (error (message "File mode specification error: %s"
-		    (prin1-to-string err))))
-  (condition-case err
-      (let ((enable-local-variables (or (not find-file)
-					enable-local-variables)))
-	(hack-local-variables))
-    (error (message "File local-variables error: %s"
-		    (prin1-to-string err))))
+  (with-errors-caught "File mode specification error: %s"
+    (set-auto-mode))
+  (with-errors-caught "File local-variables error: %s"
+    (let ((enable-local-variables (or (not find-file) enable-local-variables)))
+      (hack-local-variables)))
   (if (fboundp 'ucs-set-table-for-input) ; don't lose when building
       (ucs-set-table-for-input)))
 

4 - Don't ask for confirmation when the user has just gone through the
    trouble of typing the whole M-x revert-buffer RET thing.

@@ -3449,7 +3475,10 @@
   ;; there's no straightforward way to encourage authors to notice a
   ;; reversal of the argument sense.  So I'm just changing the user
   ;; interface, but leaving the programmatic interface the same.
-  (interactive (list (not current-prefix-arg)))
+  (interactive (list (not current-prefix-arg)
+		     ;; Don't request confirmation if the user just hit
+		     ;; M-x revert-buffer RET.
+		     (eq last-command-char ?\r)))
   (if revert-buffer-function
       (funcall revert-buffer-function ignore-auto noconfirm)
     (let* ((auto-save-p (and (not ignore-auto)


5 - Don't junk the buffer-undo-list when reverting while preserving modes.
    This is convenient with things like auto-revert or VC, especially
    combined with the undo-in-region feature.  It does require changes
    in the C code for insert-file-contents as well, since it
    currently always junks the undo list.  These changes aren't
    included here, but they are not needed for the code to work
    as well as before.

@@ -3481,20 +3510,22 @@
 		  (not (verify-visited-file-modtime (current-buffer)))
 		  (setq buffer-backed-up nil))
 	     ;; Get rid of all undo records for this buffer.
-	     (or (eq buffer-undo-list t)
+	     (or (eq buffer-undo-list t) preserve-modes
 		 (setq buffer-undo-list nil))
 	     ;; Effectively copy the after-revert-hook status,
 	     ;; since after-find-file will clobber it.
-	     (let ((global-hook (default-value 'after-revert-hook))
+	     (let* ((global-hook (default-value 'after-revert-hook))
 		   (local-hook-p (local-variable-p 'after-revert-hook))
-		   (local-hook (and (local-variable-p 'after-revert-hook)
-				    after-revert-hook)))
-	       (let (buffer-read-only
-		     ;; Don't make undo records for the reversion.
-		     (buffer-undo-list t))
+		    (local-hook (and local-hook-p after-revert-hook)))
+	       (let ((inhibit-read-only t))
 		 (if revert-buffer-insert-file-contents-function
+		     (if preserve-modes
 		     (funcall revert-buffer-insert-file-contents-function
 			      file-name auto-save-p)
+		       ;; Don't make undo records for the reversion.
+		       (let ((buffer-undo-list t))
+			 (funcall revert-buffer-insert-file-contents-function
+				  file-name auto-save-p)))
 		   (if (not (file-exists-p file-name))
 		       (error (if buffer-file-number
 				  "File %s no longer exists!"
@@ -3523,8 +3554,10 @@
 			 (let ((buffer-file-format buffer-file-format))
 			   (insert-file-contents file-name (not auto-save-p)
 						 nil nil t))
+		       ;; Don't make undo records for the reversion.
+		       (let ((buffer-undo-list t))
 		       (insert-file-contents file-name (not auto-save-p)
-					     nil nil t)))))
+					       nil nil t))))))
 	       ;; Recompute the truename in case changes in symlinks
 	       ;; have changed the truename.
 	       (setq buffer-file-truename


Any comment ?


	Stefan

Re: 4 minor suggestions for files.el

[Kai Großjohann]

I somehow don't really feel qualified to comment, but I'll try my
best, anyway.

"Stefan Monnier" <monnier+gnu/emacs@rum.cs.yale.edu> writes:

> 1 - Don't allow viewing non-existent files.  The interactive spec already
>     ensures that's the case, but it doesn't work when the function
>     is called from dired or somesuch (the file might have existed
>     when the dired buffer was created).

Nice.

> 2 - Prompt the user when trying to open a very large file.  My Emacs crawls
>     to a near halt when I try to open a 16MB file, so if I ever try to
>     open such a file, I'd rather be warned.

Maybe add a group and a type to the defcustom.

Otherwise, me likee.

> 3 - Don't catch errors to turn them into messages when debug-on-error
>     is set to t.  I wish I could solve it more generically.

I wonder what happens with debug-on-signal?  If that catches the
errors, then maybe your change isn't necessary.

If that does not catch the errors, you might extend your code to
check debug-on-signal, as well.

As a different approach, maybe it is useful to somehow make
condition-case easier to debug?  For example, one could design a
"debugging mode" which turns on debug-on-error, perhaps
debug-on-signal, and also does something special so that
condition-case invokes the debugger in case an error happened.

> 4 - Don't ask for confirmation when the user has just gone through the
>     trouble of typing the whole M-x revert-buffer RET thing.

Good idea, but will it work if the user binds it to C-c RET, say?  (I
know, that's not allowed, but perhaps they chose C-c f RET.)

> 5 - Don't junk the buffer-undo-list when reverting while preserving modes.
>     This is convenient with things like auto-revert or VC, especially
>     combined with the undo-in-region feature.  It does require changes
>     in the C code for insert-file-contents as well, since it
>     currently always junks the undo list.  These changes aren't
>     included here, but they are not needed for the code to work
>     as well as before.

Nice.

-- 
file-error; Data: (Opening input file no such file or directory ~/.signature)

Re: 4 minor suggestions for files.el

[Kevin Rodgers]

Stefan Monnier wrote:

> 2 - Prompt the user when trying to open a very large file.  My Emacs crawls
>     to a near halt when I try to open a 16MB file, so if I ever try to
>     open such a file, I'd rather be warned.
> 
> @@ -1163,6 +1166,9 @@
>    :version "21.1"
>    :type 'boolean)
>  
> +(defcustom large-file-warning-threshold 10000000
> +  "Maximum size of file above which a confirmation is requested.")
> +


Why is the default 9.537 MB?  How about something like

	(* 16 1024 1024)	; 16 MB

-- 
<a href="mailto:&lt;kevin.rodgers&#64;ihs.com&gt;">Kevin Rodgers</a>

Re: 4 minor suggestions for files.el

[Richard Stallman]

Ideas 1, 2 and 3 are good ideas--please install them.

However, I think that the name with-errors-caught is unnatural;
please call it report-errors.

Please don't change the argument to revert-buffer.
It is intentional that M-x revert-buffer RET asks for confirmation.

Idea 5 would be nice in principle, but it cannot possibly
work right.  There is no way to connect the locations
in the undo list with the new buffer contents correctly.
It would be an arbitrary decision, and the results of undoing
those changes would be nonsensical.

The only case in which this would really work right is when the buffer
text is unchanged.  That case may occur often in using VC.  Perhaps
you can make insert-file-contents detect it.  insert-file-contents
already tries to notice when text at the start of the buffer and/or
text at the end of the buffer are unchanged.  It could preserve the
undo list when the stuff to undo is within the unchanged areas
of the text at the beginning or the end.

Re: 4 minor suggestions for files.el

[Stefan Monnier]

> Ideas 1, 2 and 3 are good ideas--please install them.

Done.  People who had ideas for how to improve them or
change the defaults should feel free to do it.
(except for the :group argument which should be unnecessary)

> However, I think that the name with-errors-caught is unnatural;
> please call it report-errors.

Done.

> Please don't change the argument to revert-buffer.
> It is intentional that M-x revert-buffer RET asks for confirmation.

What is the intention exactly ?
The reaosn why I don't understand is that it is not very common
for the user to hit M-x revert-buffer RET and the only times I've
seen this happen is when the user knows full well the confirmation
is going to pop up and she'll answer "yes".

> Idea 5 would be nice in principle, but it cannot possibly
> work right.  There is no way to connect the locations
> in the undo list with the new buffer contents correctly.
> It would be an arbitrary decision, and the results of undoing
> those changes would be nonsensical.

Huh?  It works just dandy here.  The undo-log is correct (and has
the necessary entries corresponding to the insert-file operation).
The only real issue is not correctness but performance: the undo-log
might contain a very large string (if the insert-file operation had to
modify a large part of the buffer).

> The only case in which this would really work right is when the buffer
> text is unchanged.  That case may occur often in using VC.  Perhaps
> you can make insert-file-contents detect it.  insert-file-contents
> already tries to notice when text at the start of the buffer and/or
> text at the end of the buffer are unchanged.  It could preserve the
> undo list when the stuff to undo is within the unchanged areas
> of the text at the beginning or the end.

I was wondering if it would be worth it to try and make insert-file
even more clever: pass the `before' and `after' to diff and then apply
the diff by hand.  This will preserve even more of the markers
(and would make the undo-log smaller and more useful, in the case
that revert-buffer doesn't flush it).


	Stefan

Re: 4 minor suggestions for files.el

[Richard Stallman]

    Huh?  It works just dandy here.  The undo-log is correct (and has
    the necessary entries corresponding to the insert-file operation).
    The only real issue is not correctness but performance: the undo-log
    might contain a very large string (if the insert-file operation had to
    modify a large part of the buffer).

I think I understand now--it treats the insert-file operation as an
ordinary undoable buffer change.

Spiegel, is that really the right thing to do in VC?

I also worry about the large strings.  Maybe there should be a file
size threshold on this change.

    I was wondering if it would be worth it to try and make insert-file
    even more clever: pass the `before' and `after' to diff and then apply
    the diff by hand.  This will preserve even more of the markers
    (and would make the undo-log smaller and more useful, in the case
    that revert-buffer doesn't flush it).

That is a very interesting idea.  Perhaps diff could have an output
format designed to give the best possible results with Emacs, and
for the highest efficiency.

Re: 4 minor suggestions for files.el

[Andre Spiegel]

On Fri, 2003-04-18 at 13:28, Richard Stallman wrote:

> I think I understand now--it treats the insert-file operation as an
> ordinary undoable buffer change.
> 
> Spiegel, is that really the right thing to do in VC?

VC does a revert-buffer after each version control operation, so that
changes in version headers are picked up.

If I understand correctly, the revert-buffer would be undoable.

This may be confusing because the actual version control operation would
not be undone.  After undoing the revert-buffer, the version headers
would no longer correspond to the version control state of the file.

If the feature is perceived generally useful, I think VC ought to clear
the undo list by itself in each version control operation.

Re: 4 minor suggestions for files.el

[Stefan Monnier]

> > I think I understand now--it treats the insert-file operation as an
> > ordinary undoable buffer change.
> > 
> > Spiegel, is that really the right thing to do in VC?
> 
> VC does a revert-buffer after each version control operation, so that
> changes in version headers are picked up.
> 
> If I understand correctly, the revert-buffer would be undoable.
> 
> This may be confusing because the actual version control operation would
> not be undone.  After undoing the revert-buffer, the version headers
> would no longer correspond to the version control state of the file.
> 
> If the feature is perceived generally useful, I think VC ought to clear
> the undo list by itself in each version control operation.

I strongly disagree.  I went to the trouble of writing the patch
and I almost exclusively use it with VC (though sometimes
also with auto-revert-mode).

Why would it hurt particularly to undo the version-headers-change ?


	Stefan

Re: 4 minor suggestions for files.el

[Andre Spiegel]

On Fri, 2003-04-18 at 16:23, Stefan Monnier wrote:

> Why would it hurt particularly to undo the version-headers-change ?

With RCS, for example, VC uses the version header to determine the file
version and file state, as displayed in the mode line.  If you undo the
version header change, save and revisit, VC will show a wrong version,
and it might think that the file is locked when in fact it isn't, etc.

Re: 4 minor suggestions for files.el

[Stefan Monnier]

> > Why would it hurt particularly to undo the version-headers-change ?
> 
> With RCS, for example, VC uses the version header to determine the file
> version and file state, as displayed in the mode line.  If you undo the
> version header change, save and revisit, VC will show a wrong version,
> and it might think that the file is locked when in fact it isn't, etc.

But this is a quirk of RCS.  We could do something specific
for RCS, but there's no reason why CVS, Subversion, SCCS and others
should suffer from it as well.


	Stefan

Re: 4 minor suggestions for files.el

[Stefan Monnier]

> > Why would it hurt particularly to undo the version-headers-change ?
> 
> With RCS, for example, VC uses the version header to determine the file
> version and file state, as displayed in the mode line.  If you undo the
> version header change, save and revisit, VC will show a wrong version,
> and it might think that the file is locked when in fact it isn't, etc.

BTW.  It seems this problem is not specific to revert-buffer and undo.
Shouldn't we mark the $Id$ version header read-only ?


	Stefan

Re: 4 minor suggestions for files.el

[Richard Stallman]

    BTW.  It seems this problem is not specific to revert-buffer and undo.

It partly is and partly isn't.  Yes, you can mistakenly edit the
version header with any commands.  But what's specific to undoing the
revert is that you are particularly likely to edit the version header
that way, and not even know you are doing so.

    Shouldn't we mark the $Id$ version header read-only ?

Then you could not delete the header.

Re: 4 minor suggestions for files.el

[Richard Stallman]

    VC does a revert-buffer after each version control operation, so that
    changes in version headers are picked up.

    If I understand correctly, the revert-buffer would be undoable.

I think that is what he is proposing.

I think that is a misguided feature.  It is not right to undo
the revert-buffer unless you can undo the whole VC operation with it.
But that, if we can implement it, must be a VC operation;
M-x undo should not be able to do it.

I think that the correct way for check-in and update to interact with
M-x undo is to transpose the old undo list somehow to the buffer as it
has been updated.

If I edit the buffer and then do an RCS check-in, and the check-in
updates an RCS header in the file, and then I undo, this should not
undo the change in the RCS header.  Rather, it should undo the change
that I made manually before the check-in.

Likewise, if I edit the buffer and then do a CVS update, I should
then be able to undo the edit I made, but not the changes made by
the CVS update.

Implementing this is nontrivial, but not impossible.

The revert-buffer should never be undone without (say

Re: 4 minor suggestions for files.el

[Stefan Monnier]

>     VC does a revert-buffer after each version control operation, so that
>     changes in version headers are picked up.
> 
>     If I understand correctly, the revert-buffer would be undoable.
> 
> I think that is what he is proposing.
> 
> I think that is a misguided feature.  It is not right to undo
> the revert-buffer unless you can undo the whole VC operation with it.

Why?  Especially with `undo-in-region' it doesn't make sense to
undo the operation when only part of the revert is undone (or
as is more often the case for me: when only changes prior to the
revert are undone).

> But that, if we can implement it, must be a VC operation;
> M-x undo should not be able to do it.

I don't see why not.  Just like we can undo changes that were
saved with C-x C-s, it makes perfect sense to undo changes
that were made by a "minor" revert-buffer (my patch defines
"minor" as "which preserves minor-modes").

> I think that the correct way for check-in and update to interact with
> M-x undo is to transpose the old undo list somehow to the buffer as it
> has been updated.

That's what undo-in-region does.

> If I edit the buffer and then do an RCS check-in, and the check-in
> updates an RCS header in the file, and then I undo, this should not
> undo the change in the RCS header.  Rather, it should undo the change
> that I made manually before the check-in.

Just because there's one case where care might be needed, we shouldn't
prevent people for whom this doesn't apply to benefit from more
flexibility.

> Likewise, if I edit the buffer and then do a CVS update, I should
> then be able to undo the edit I made, but not the changes made by
> the CVS update.

Why not ?


	Stefan

Re: 4 minor suggestions for files.el

[Richard Stallman]

    > I think that the correct way for check-in and update to interact with
    > M-x undo is to transpose the old undo list somehow to the buffer as it
    > has been updated.

    That's what undo-in-region does.

undo-in-region does a special case of this.  It does not handle the
generality that would be needed for this, and I think that would require
new methods.

    > Likewise, if I edit the buffer and then do a CVS update, I should
    > then be able to undo the edit I made, but not the changes made by
    > the CVS update.

    Why not ?

When you do a CVS update, you don't know what changes are getting
merged in.  You often don't see them; they are in parts of the file
where you have not been working.

If a single undo operation would undo all of them, you would lose
them and not know what you are losing.  That is risky and not the
right interface.

In order for undo in Emacs to operate in a predictable and reliable
way, it should undo the changes that you made with your editing.  If
you want to undo the changes that came from CVS, that should be a
separate command, designed to coordinate with CVS.  What I have in
mind is a command to take out the changes that were made between a
certain pair of versions.

Re: 4 minor suggestions for files.el

[Andre Spiegel]

On Mon, 2003-05-05 at 16:32, Richard Stallman wrote:

> What I have in mind is a command to take out the changes that 
> were made between a certain pair of versions.

We already have that.  vc-merge (C-x v m) can do it; just specify the
version numbers in reverse.  E.g. typing C-x v m 1.4 RET 1.2 RET takes
out the changes that were made from version 1.2 to 1.4.

Re: 4 minor suggestions for files.el

[Richard Stallman]

    We already have that.  vc-merge (C-x v m) can do it; just specify the
    version numbers in reverse.  E.g. typing C-x v m 1.4 RET 1.2 RET takes
    out the changes that were made from version 1.2 to 1.4.

(I didn't know about vc-merge.)

For vc-merge, since that is an explicit command to apply a patch to
the buffer, I think it would be right for the patch to be undoable.
Is it undoable now?  Could it be made so?  Would that require a change
somewhat like the one Stephan proposed?

Re: 4 minor suggestions for files.el

[Andre Spiegel]

On Tue, 2003-05-06 at 12:14, Richard Stallman wrote:

> For vc-merge, since that is an explicit command to apply a patch to
> the buffer, I think it would be right for the patch to be undoable.
> Is it undoable now?  Could it be made so?  Would that require a change
> somewhat like the one Stephan proposed?

You can always use the symmetric operation, of course.  You can take out
the changes from 1.2 to 1.4 using vc-merge, and then you can reapply
them by simply reversing the arguments again.

I don't think it's wise to make this available through the general undo
mechanism.  For example, vc-merge may also detect conflicts with your
own changes, and you need to resolve them before the merge operation is
actually finished.  How would you undo that?  This is getting very
complicated, and I think it's wiser just to provide the user with
symmetric operations.

Re: 4 minor suggestions for files.el

[Richard Stallman]

    I don't think it's wise to make this available through the general undo
    mechanism.  For example, vc-merge may also detect conflicts with your
    own changes, and you need to resolve them before the merge operation is
    actually finished.  How would you undo that?

I presume you would resolve them through editing, so I guess you
could undo those editing operations.  Would that be unreasonable?

Re: 4 minor suggestions for files.el

[Andre Spiegel]

On Wed, 2003-05-07 at 13:51, Richard Stallman wrote:

>     I don't think it's wise to make this available through the general undo
>     mechanism.  For example, vc-merge may also detect conflicts with your
>     own changes, and you need to resolve them before the merge operation is
>     actually finished.  How would you undo that?
> 
> I presume you would resolve them through editing, so I guess you
> could undo those editing operations.  Would that be unreasonable?

Conflict resolution works by invoking smerge-mode or ediff after the
merge operation, automatically.  If we follow the undo path, we ought to
make sure what the interactions will be with these mechanisms.

Stefan?

Re: 4 minor suggestions for files.el

[Stefan Monnier]

> >     I don't think it's wise to make this available through the general undo
> >     mechanism.  For example, vc-merge may also detect conflicts with your
> >     own changes, and you need to resolve them before the merge operation is
> >     actually finished.  How would you undo that?
> > 
> > I presume you would resolve them through editing, so I guess you
> > could undo those editing operations.  Would that be unreasonable?
> 
> Conflict resolution works by invoking smerge-mode or ediff after the
> merge operation, automatically.  If we follow the undo path, we ought to
> make sure what the interactions will be with these mechanisms.

I don't understand the question: `undo' is a normal editing operation,
just like any other, and smerge also doesn't do anything special about
the way it edits the buffer.  So they just interact in the obvious
and expected way.
The only thing that might be relevant is that if you resolve the last
conflict and turn off smerge-mode, undoing the last change won't
automatically turn smerge-mode back on.  It would be nice to improve
this, but it's really not that important.


	Stefan

Re: 4 minor suggestions for files.el

[Richard Stallman]

    > I presume you would resolve them through editing, so I guess you
    > could undo those editing operations.  Would that be unreasonable?

    Conflict resolution works by invoking smerge-mode or ediff after the
    merge operation, automatically.  If we follow the undo path, we ought to
    make sure what the interactions will be with these mechanisms.

I think each interaction should be undone as a single unit.
This will happen if calls to undo-boundary are at the right places.

Re: 4 minor suggestions for files.el

[Stefan Monnier]

>     We already have that.  vc-merge (C-x v m) can do it; just specify the
>     version numbers in reverse.  E.g. typing C-x v m 1.4 RET 1.2 RET takes
>     out the changes that were made from version 1.2 to 1.4.
> 
> (I didn't know about vc-merge.)
> 
> For vc-merge, since that is an explicit command to apply a patch to
> the buffer, I think it would be right for the patch to be undoable.
> Is it undoable now?

No.

> Could it be made so?  Would that require a change
> somewhat like the one Stefan proposed?

Yes, indeed.
My suggested change indeed allows the change to be undone.
The reason is the following: vc-merge works on the file rather than
on the buffer, so after the merge happens, VC does a revert-buffer
(unless the buffer was modified), so my suggested change allows
you to undo it because it can undo the revert-buffer.


	Stefan

Re: 4 minor suggestions for files.el

[Richard Stallman]

    > Could it be made so?  Would that require a change
    > somewhat like the one Stefan proposed?

    Yes, indeed.
    My suggested change indeed allows the change to be undone.
    The reason is the following: vc-merge works on the file rather than
    on the buffer, so after the merge happens, VC does a revert-buffer
    (unless the buffer was modified), so my suggested change allows
    you to undo it because it can undo the revert-buffer.

Perhaps we should make your change, but do it as an option,
not unconditionally.  Then we could make vc-merge specify that option.

Re: 4 minor suggestions for files.el

[Stefan Monnier]

>     > I think that the correct way for check-in and update to interact with
>     > M-x undo is to transpose the old undo list somehow to the buffer as it
>     > has been updated.
> 
>     That's what undo-in-region does.
> 
> undo-in-region does a special case of this.  It does not handle the
> generality that would be needed for this, and I think that would require
> new methods.

Sure, it's not quite the same, but I use it to get the desired result.

>     > Likewise, if I edit the buffer and then do a CVS update, I should
>     > then be able to undo the edit I made, but not the changes made by
>     > the CVS update.
> 
>     Why not ?
> 
> When you do a CVS update, you don't know what changes are getting
> merged in.  You often don't see them; they are in parts of the file
> where you have not been working.
> 
> If a single undo operation would undo all of them, you would lose
> them and not know what you are losing.  That is risky and not the
> right interface.
> 
> In order for undo in Emacs to operate in a predictable and reliable
> way, it should undo the changes that you made with your editing.  If
> you want to undo the changes that came from CVS, that should be a
> separate command, designed to coordinate with CVS.  What I have in
> mind is a command to take out the changes that were made between a
> certain pair of versions.

CVS generally doesn't know: the local file generally has local
modifications (otherwise, there's generally no interesting undo log
to worry about) so the merge done by `cvs update' is not the same
as the change recorded in the repository.

Also, most of the time that I use such an undo, it's only to temporarily
take a look at a previous state of the buffer: I virtually never
want to save the file with the change undone.  The only exception
is when I want to undo (in the repository) a change I installed
recently, but I rarely if ever use Emacs undo for it (I use `cvs diff'
and reverse-apply the patch, instead).

I.e. in my experience, undoing the revert (rather than only undoing changes
done before the revert) never hurts and is sometimes useful.  I see
cases where your refinement (i.e. adjusting the "before revert" undo
log so it applies to the "after revert" buffer) can be marginally better,
but I doubt it's worth the effort, especially since it prevents the
user from undoing the revert itself which can also be something she wants
to do.


	Stefan

Re: 4 minor suggestions for files.el

[Richard Stallman]

    > In order for undo in Emacs to operate in a predictable and reliable
    > way, it should undo the changes that you made with your editing.  If
    > you want to undo the changes that came from CVS, that should be a
    > separate command, designed to coordinate with CVS.  What I have in
    > mind is a command to take out the changes that were made between a
    > certain pair of versions.

    CVS generally doesn't know: the local file generally has local
    modifications (otherwise, there's generally no interesting undo log
    to worry about) so the merge done by `cvs update' is not the same
    as the change recorded in the repository.

Are you saying vc-merge does not do this job?

Re: 4 minor suggestions for files.el

[Stefan Monnier]

>     > In order for undo in Emacs to operate in a predictable and reliable
>     > way, it should undo the changes that you made with your editing.  If
>     > you want to undo the changes that came from CVS, that should be a
>     > separate command, designed to coordinate with CVS.  What I have in
>     > mind is a command to take out the changes that were made between a
>     > certain pair of versions.
> 
>     CVS generally doesn't know: the local file generally has local
>     modifications (otherwise, there's generally no interesting undo log
>     to worry about) so the merge done by `cvs update' is not the same
>     as the change recorded in the repository.
> 
> Are you saying vc-merge does not do this job?

Not quite.  Just like

    patch file <foo.diff
    patch -R file <foo.diff

does not always return the original file.


	Stefan

Re: 4 minor suggestions for files.el

[Richard Stallman]

    Not quite.  Just like

	patch file <foo.diff
	patch -R file <foo.diff

    does not always return the original file.

Can you tell me when it doesn't?  I use CVS nowadays but I am no
expert.