[PATCH] show difference when recovering file

[PATCH] show difference when recovering file

[Ivan Kanis]

Hi,

This trivial patch offer to see the difference between the current file
and the backup file. This helps making the decision whether one wants to
recover the file.
   

--- src/files.el        2012-12-16 17:49:04.926869000 +0100
+++ ivan/files.el       2012-12-16 17:50:12.051423000 +0100
@@ -5370,6 +5370,7 @@
            #'(lambda (window _value)
                (with-selected-window window
                  (unwind-protect
+                      (diff file file-name)
                      (yes-or-no-p (format "Recover auto save file %s? " file-name))
                    (when (window-live-p window)
                      (quit-restore-window window 'kill)))))
-- 
Ivan Kanis
http://ivan.kanis.fr

When a man is attempting to describe another person's character, he
may be right or he may be wrong, but in one thing he will always
succeed,that is, in describing himself.
    -- Samuel Taylor Coleridge

Re: [PATCH] show difference when recovering file

[martin rudalics]

>                   (unwind-protect
> +                      (diff file file-name)
>                       (yes-or-no-p (format "Recover auto save file %s? " file-name))
>                     (when (window-live-p window)
>                       (quit-restore-window window 'kill)))))

Do you really want to ask the `yes-or-no-p' in the unwind form?

martin

Re: [PATCH] show difference when recovering file

[Stefan Monnier]

> This trivial patch offer to see the difference between the current file
> and the backup file. This helps making the decision whether one wants to
> recover the file.
   
> +                      (diff file file-name)

There's a problem here: the autosave file is saved using the internal
coding-system (so it always succeeds, no matter what weird mix of
characters happens to be currently in the buffer), where the
non-autosave file will use the encoding which happens to be in use for
this file.
So the diff may show lots of spurious changes.


        Stefan

Re: [PATCH] show difference when recovering file

[Juri Linkov]

> This trivial patch offer to see the difference between the current file
> and the backup file. This helps making the decision whether one wants to
> recover the file.

`save-some-buffers' provides the key `d' in `save-some-buffers-action-alist'
to do almost the same.

Re: [PATCH] show difference when recovering file

[Ivan Kanis]

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

>> This trivial patch offer to see the difference between the current file
>> and the backup file. This helps making the decision whether one wants to
>> recover the file.
>    
>> +                      (diff file file-name)
>
> There's a problem here: the autosave file is saved using the internal
> coding-system (so it always succeeds, no matter what weird mix of
> characters happens to be currently in the buffer), where the
> non-autosave file will use the encoding which happens to be in use for
> this file.

Is it possible to open the non-autosave file in the internal format just
to run the diff? I guess not.

> So the diff may show lots of spurious changes.

I think it's a borderline case. It's more useful to see what the
difference is between the two files even though on rare occasion the
diff will show garbage
-- 
Ivan Kanis
http://ivan.kanis.fr

I had an interest in death from an early age. It fascinated me. When I
heard 'Humpty Dumpty sat on a wall,' I thought, 'Did he fall or was
he pushed?'
    -- P.D. James

Re: [PATCH] show difference when recovering file

[Ivan Kanis]

Juri Linkov <juri@jurta.org> wrote:

>> This trivial patch offer to see the difference between the current file
>> and the backup file. This helps making the decision whether one wants to
>> recover the file.
>
> `save-some-buffers' provides the key `d' in `save-some-buffers-action-alist'
> to do almost the same.

Do you propose I take that code in a function?

The idea is that it will be called by 'save-some-buffer' and
'recover-file'. That would get rid of the duplication
-- 
Ivan Kanis
http://ivan.kanis.fr

O Love, O fire! Once he drew
With one long kiss my whole soul through
My lips, as sunlight drinketh dew.
    -- Alfred, Lord Tennyson

Re: [PATCH] show difference when recovering file

[Stefan Monnier]

>> There's a problem here: the autosave file is saved using the internal
>> coding-system (so it always succeeds, no matter what weird mix of
>> characters happens to be currently in the buffer), where the
>> non-autosave file will use the encoding which happens to be in use for
>> this file.
> Is it possible to open the non-autosave file in the internal format just
> to run the diff? I guess not.

There's no "open"ing involved: the two files are passed to `diff' and
that's that.  So, to fix the problem, we have to load the non-autoload
file, save it using the internal-encoding into a new temporary file, and
then pass this file to diff.

BTW, the "save-to-temp and then diff" is called `diff-buffer-with-file'.

>> So the diff may show lots of spurious changes.
> I think it's a borderline case. It's more useful to see what the
> difference is between the two files even though on rare occasion the
> diff will show garbage

I'd rather do it right.

BTW, the way the user can do it currently, is to say "yes" to recover,
and then use M-x diff-buffer-with-file RET to decide whether or not
recovering is a good idea.

Rather than always popping up a *diff* buffer, we could also change the
prompt from "yes/no" to "yes/no/diff".


        Stefan

Re: [PATCH] show difference when recovering file

[Ivan Kanis]

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

>>> There's a problem here: the autosave file is saved using the internal
>>> coding-system (so it always succeeds, no matter what weird mix of
>>> characters happens to be currently in the buffer), where the
>>> non-autosave file will use the encoding which happens to be in use for
>>> this file.
>> Is it possible to open the non-autosave file in the internal format just
>> to run the diff? I guess not.
>
> There's no "open"ing involved: the two files are passed to `diff' and
> that's that.  So, to fix the problem, we have to load the non-autoload
> file, save it using the internal-encoding into a new temporary file, and
> then pass this file to diff.
>
> BTW, the "save-to-temp and then diff" is called `diff-buffer-with-file'.
>

OK I understand.

>>> So the diff may show lots of spurious changes.
>> I think it's a borderline case. It's more useful to see what the
>> difference is between the two files even though on rare occasion the
>> diff will show garbage
>
> I'd rather do it right.

Me too.

> BTW, the way the user can do it currently, is to say "yes" to recover,
> and then use M-x diff-buffer-with-file RET to decide whether or not
> recovering is a good idea.

It's too convoluted for me, I never remember how to do it when I need it.

> Rather than always popping up a *diff* buffer, we could also change the
> prompt from "yes/no" to "yes/no/diff".

We could have the same UI as implemented in
'save-some-buffers-action-alist' one key for:

y yes
n no
d for diff

Another way would be to always kill the diff buffer after recovering.

Let me know what UI you like best.

Take care,

Ivan
-- 
The road up and the road down are one and the same.
    -- Heraclitus

Re: [PATCH] show difference when recovering file

[Wolfgang Jenkner]

On Mon, Dec 17 2012, Stefan Monnier wrote:

> Rather than always popping up a *diff* buffer, we could also change the
> prompt from "yes/no" to "yes/no/diff".

Instead of surprising the user with an additional choice, how about
putting the *Directory* buffer in dired-mode, via dired-virtual from the
dired-x package (which comes with emacs, after all).  The user could
than use = or whatever in the usual way (though the default is to diff
with a backup-file instead of an auto-save file, but that's a SMOP).

Proof of concept below, just to convey the idea.

Wolfgang

=== modified file 'lisp/files.el'
--- lisp/files.el	2012-12-14 18:59:00 +0000
+++ lisp/files.el	2012-12-17 16:57:10 +0000
@@ -5371,6 +5371,8 @@
   (interactive)
   (recover-file buffer-file-name))
 
+(require 'dired-x)
+
 (defun recover-file (file)
   "Visit file FILE, but get contents from its last auto-save file."
   ;; Actually putting the file name in the minibuffer should be used
@@ -5406,7 +5408,8 @@
 		;; The code in v20.x called `ls' directly, so we need
 		;; to emulate what `ls' did in that case.
 		(insert-directory-safely file switches)
-		(insert-directory-safely file-name switches))))
+		(insert-directory-safely file-name switches))
+	      (dired-virtual (file-name-directory file))))
 	   (switch-to-buffer (find-file-noselect file t))
 	   (let ((inhibit-read-only t)
 		 ;; Keep the current buffer-file-coding-system.

Re: [PATCH] show difference when recovering file

[Achim Gratz]

Stefan Monnier writes:
> Rather than always popping up a *diff* buffer, we could also change the
> prompt from "yes/no" to "yes/no/diff".

+1

Incidentally, this is how aptitude asks for your decision when a config
file needs to replaced that has been edited locally.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptation for Waldorf Blofeld V1.15B11:
http://Synth.Stromeko.net/Downloads.html#WaldorfSDada

Re: [PATCH] show difference when recovering file

[Stefan Monnier]

>> BTW, the way the user can do it currently, is to say "yes" to recover,
>> and then use M-x diff-buffer-with-file RET to decide whether or not
>> recovering is a good idea.
> It's too convoluted for me, I never remember how to do it when I need it.

I agree it's not very discoverable and it's rare enough to be hard to
remember.

>> Rather than always popping up a *diff* buffer, we could also change the
>> prompt from "yes/no" to "yes/no/diff".
> We could have the same UI as implemented in
> 'save-some-buffers-action-alist' one key for:
> y yes
> n no
> d for diff

Right, that's the kind of thing I had in mind, indeed.
I'm wondering why we have yes/no instead of y/n right now, tho.


        Stefan

Re: [PATCH] show difference when recovering file

[Ivan Kanis]

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

> Right, that's the kind of thing I had in mind, indeed.

I'll work on it after the release. It's been on my 'todo' for a while
now...
-- 
Ivan Kanis
http://ivan.kanis.fr

The greatest pleasure I know, is to do a good action by stealth, and
to have it found out by accident.
    -- Charles Lamb

I am listening to "Audio Bullys - 100 Million".

Re: [PATCH] show difference when recovering file

[Stephen J. Turnbull]

Wolfgang Jenkner writes:
 > On Mon, Dec 17 2012, Stefan Monnier wrote:
 > 
 > > Rather than always popping up a *diff* buffer, we could also change the
 > > prompt from "yes/no" to "yes/no/diff".
 > 
 > Instead of surprising the user with an additional choice,

FWIW, no XEmacs user has complained about the additional choice in 10 years.