Re: Improvements to Recover Emergency File prompts. (pseudo-patch with discussion)

[Pavel Sanda]

John McCabe-Dansted wrote:
  yes there was bug about it was and we fixed it by the dialog obove not so 
  long ago.
  constant re-asking again about recovering the file until one gets deep down 
  in
  dir structure where the file is located.
 
 By constant re-asking, do you mean asking each time LyX starts?

yes

 Would a comment on the dialog saying To prevent this dialog being
displayed in future, please save this document suffice?

if i'm minority then its better than nothing.

pavel

Re: Improvements to "Recover Emergency File" prompts. (pseudo-patch with discussion)

[Pavel Sanda]

John McCabe-Dansted wrote:
> > yes there was bug about it was and we fixed it by the dialog obove not so 
> > long ago.
> > constant re-asking again about recovering the file until one gets deep down 
> > in
> > dir structure where the file is located.
> 
> By constant re-asking, do you mean asking each time LyX starts?

yes

> Would a comment on the dialog saying "To prevent this dialog being
>displayed in future, please save this document" suffice?

if i'm minority then its better than nothing.

pavel

Improvements to Recover Emergency File prompts. (pseudo-patch with discussion)

[John McCabe-Dansted]

I've been using a number of changes to Buffer::readFileHelper that
I've found useful. I err on the side off not delete emergency files,
and also add a Recover All option that is useful when you have a
master document that has several modified child documents.

I think that these modifications require a little discussion before
being formally proposed, so I discuss them inline rather than submit
them as a formal patch.

--
switch (Alert::prompt(_(Load backup?), text, 0, 2,
  _(Load backup), _(Load original),
  _(Cancel) ))
{
...
case 1:
-   // Here we delete the autosave
-   a.removeFile();

I don't see why we delete the autosave here. What I sometimes do is think:
   OK, I was doing some experimental stuff. I should probably open
the original file. Opps, I didn't do a proper save in the last hour,
never-mind I'll just restart LyX and pick load Backup. Huh, my
autosave file and all my work is gone!

Simply removing that line prevents that problem, and doesn't seem to
cause any significant regressions.

If we want to automatically delete the autosave file, I'd do it once
we have verified that the file is old and obsolete. E.g. something
like the psuedo-code:

IF auto-save is newer
Prompt user etc.
ELSE
delete auto-save.

-

-   if (!Alert::prompt(_(Delete emergency file?), str, 1, 
1,
-   _(Remove), _(Keep it))) {
-  ...

Well, here at least we warn the user that we are about to delete
stuff. However, when LyX crashes the only thing on my mind is getting
my data back as quickly as possible. I think we should just keep the
emergency file (unless it is obsolete). We could possibly ask the user
this question on shutdown.

Otherwise I'd add a Keep All option.

-
-   if (e.exists()  s.exists()  e.lastModified()  s.lastModified()) {
+
+   if (e.exists()  s.exists()  e.lastModified()  s.lastModified() 
+e.checksum() != s.checksum()) {

There are a number of ways to get a dirty buffer without actually
changing the file. E.g we can add two spaces that get converted to a
single space, or we can add a character and then remove it by
backspacing. This seems to happen fairly regularly to me, so it seems
worth checking for.

However I am not sure .checksum() is the best way to do it. The
checksum is a long, so even if we assume the checksum algorithm is
ideal, we still get a false positive once per 4 billion checks. It
seems like something like:
e.fileContents() == s.fileContents()
would be cleaner and just as fast. However that could use a lot of
memory on large files. It seems the best way would be to do:
e.fileContentsEqual(s)
where FileName::fileContentsEqual is a new method that compares the
two files in (say) 1MB chunks. However I think it would be adequate to
implement this as something like:

bool FileName::fileContentsEqual(FileName const  other_file)
{
return (d-fi.size() == other_file-fi.size() 
fileContents() == other_file-fileContents())
}

as
0) this is way simpler,
1)  it seems unlikely that we would have a single valid lyx file that
does not easily fit in memory,
2)  the file size check should stop us from loading emergency files
larger than the valid lyx file, and
3) we could always improve fileContentsEqual later if need be.

-
-   switch (Alert::prompt(_(Load emergency save?), text, 0, 2,
- _(Recover),  _(Load Original),
- _(Cancel)))
+   int ret;
+   if (recover_all) {
+   ret = 0;
+   } else {
+   ret = Alert::prompt(_(Load emergency save?),
+   text, 0, 2,
+   _(Recover),  _(Load Original),
+   _(Cancel), _(Recover All));
+   if (ret == 3) {
+   ret = 0;
+   recover_all = true;
+   }
+   }
+   switch (ret)

This is the code for the Recover All emergency save.

-- 
John C. McCabe-Dansted

Re: Improvements to Recover Emergency File prompts. (pseudo-patch with discussion)

[Richard Heck]

On 10/25/2010 03:58 AM, John McCabe-Dansted wrote:

-

-   if (!Alert::prompt(_(Delete emergency file?), str, 1, 
1,
-   _(Remove), _(Keep it))) {
-  ...

Well, here at least we warn the user that we are about to delete
stuff. However, when LyX crashes the only thing on my mind is getting
my data back as quickly as possible. I think we should just keep the
emergency file (unless it is obsolete). We could possibly ask the user
this question on shutdown.

   
I think this was added a while ago, because we were leaving emergency 
files lying around. We only ask about this if an attempt to recover the 
file has been made, either successfully or not. So either the document 
is now loaded in the buffer or else we couldn't load it.



Otherwise I'd add a Keep All option.

   

What does this mean?


-
-   if (e.exists()  s.exists()  e.lastModified()  s.lastModified()) {
+
+   if (e.exists()  s.exists()  e.lastModified()  s.lastModified()
+e.checksum() != s.checksum()) {

There are a number of ways to get a dirty buffer without actually
changing the file. E.g we can add two spaces that get converted to a
single space, or we can add a character and then remove it by
backspacing. This seems to happen fairly regularly to me, so it seems
worth checking for.

   
I'm not sure where this code is. I'd want to see the context before 
deciding whether this is worth it.



However I am not sure .checksum() is the best way to do it. The
checksum is a long, so even if we assume the checksum algorithm is
ideal, we still get a false positive once per 4 billion checks. It
seems like something like:
 e.fileContents() == s.fileContents()
would be cleaner and just as fast.

   
Just use some new function equalContents(e,s). We don't actually care 
about the contents; only if they're equal. Then you can use an 
istreambuf_iterator, as in the checksum code, and the memory issue is 
less serious. You can also return false as soon as you find a difference.



-
-   switch (Alert::prompt(_(Load emergency save?), text, 0, 2,
- _(Recover),  _(Load Original),
- _(Cancel)))
+   int ret;
+   if (recover_all) {
+   ret = 0;
+   } else {
+   ret = Alert::prompt(_(Load emergency save?),
+   text, 0, 2,
+   _(Recover),  _(Load Original),
+   _(Cancel), _(RecoverAll));
+   if (ret == 3) {
+   ret = 0;
+   recover_all = true;
+   }
+   }
+   switch (ret)

This is the code for the Recover All emergency save.

   

Can you redo this, separately, given the recent changes?

Richard

Re: Improvements to Recover Emergency File prompts. (pseudo-patch with discussion)

[John McCabe-Dansted]

On Tue, Oct 26, 2010 at 12:03 AM, Richard Heck rgh...@comcast.net wrote:
 I think this was added a while ago, because we were leaving emergency files
 lying around. We only ask about this if an attempt to recover the file has
 been made, either successfully or not. So either the document is now loaded
 in the buffer or else we couldn't load it.

Again, I'd personally put this in ::save(). Leaving an emergency file
around until the user next saves doesn't sound too bad to me. If
everything goes well the user will continue working on the file and
save it soon enough. If it doesn't go well, keeping the file around is
probably a good idea. (The user might not know whether everything will
go well at the point we ask them)

 Otherwise I'd add a Keep All option.

 What does this mean?

Once you click Keep All, we keep all the saves without prompting the user.

 +                        e.checksum() != s.checksum()) {

 There are a number of ways to get a dirty buffer without actually
 changing the file. E.g we can add two spaces that get converted to a
 single space, or we can add a character and then remove it by
 backspacing. This seems to happen fairly regularly to me, so it seems
 worth checking for.

 I'm not sure where this code is. I'd want to see the context before deciding
 whether this is worth it.

It occurs immediately prior to prompting the user whether we want to
open the original or the emergency file. I consider it worthwhile
since even IO is cheap compared to user time (and user concentration).

This is now like:
--
Buffer::ReadStatus Buffer::readEmergency(FileName const  fn)
{
   FileName const emergencyFile = getEmergencyFileNameFor(fn);
if (!emergencyFile.exists()
  || emergencyFile.lastModified() = fn.lastModified() ||
+   emergencyFile.checksum() ==  fn.checksum())
return ReadFileNotFound;

docstring const file = makeDisplayPath(fn.absFileName(), 20);


(and similar for autosave files)

 This is the code for the Recover All emergency save.

 Can you redo this, separately, given the recent changes?

Sure (attached), but note that without having an e.g. Keep All
option, if you open several files you'll still have several windows
popping up asking you if you want to keep the emergency saves.

-- 
John C. McCabe-Dansted
Index: Buffer.cpp
===
--- Buffer.cpp	(revision 35834)
+++ Buffer.cpp	(working copy)
@@ -3617,6 +3627,8 @@
 
 Buffer::ReadStatus Buffer::readEmergency(FileName const  fn)
 {
+	static bool recover_all = false;
+
 	FileName const emergencyFile = getEmergencyFileNameFor(fn);
 	if (!emergencyFile.exists() 
 		  || emergencyFile.lastModified() = fn.lastModified())
@@ -3626,9 +3638,20 @@
 	docstring const text = bformat(_(An emergency save of the document 
 		%1$s exists.\n\nRecover emergency save?), file);
 	
-	int const load_emerg = Alert::prompt(_(Load emergency save?), text,
-		0, 2, _(Recover), _(Load Original), _(Cancel));
+	int load_emerg;
 
+	if (recover_all) {
+		load_emerg = 0;
+	} else {
+		load_emerg = Alert::prompt(_(Load emergency save?), text,
+			0, 2, _(Recover), _(Load Original),
+			_(Cancel), _(Recover All));
+		if (load_emerg == 3) {
+			load_emerg = 0;
+			recover_all = true;
+		}
+	}
+
 	switch (load_emerg)
 	{
 	case 0: {

Re: Improvements to Recover Emergency File prompts. (pseudo-patch with discussion)

[Jose Quesada]

Having lost a few mins of inspired writing, I'm all for leaving the
emergency files lying around...
Best,
-Jose

Jose Quesada, PhD.
Research scientist,
Max Planck Institute,
Center for Adaptive Behavior and Cognition,
Berlin
http://www.josequesada.name/
http://twitter.com/Quesada


On Mon, Oct 25, 2010 at 7:23 PM, John McCabe-Dansted gma...@gmail.comwrote:

 On Tue, Oct 26, 2010 at 12:03 AM, Richard Heck rgh...@comcast.net wrote:
  I think this was added a while ago, because we were leaving emergency
 files
  lying around. We only ask about this if an attempt to recover the file
 has
  been made, either successfully or not. So either the document is now
 loaded
  in the buffer or else we couldn't load it.

 Again, I'd personally put this in ::save(). Leaving an emergency file
 around until the user next saves doesn't sound too bad to me. If
 everything goes well the user will continue working on the file and
 save it soon enough. If it doesn't go well, keeping the file around is
 probably a good idea. (The user might not know whether everything will
 go well at the point we ask them)

  Otherwise I'd add a Keep All option.
 
  What does this mean?

 Once you click Keep All, we keep all the saves without prompting the
 user.

  +e.checksum() != s.checksum()) {
 
  There are a number of ways to get a dirty buffer without actually
  changing the file. E.g we can add two spaces that get converted to a
  single space, or we can add a character and then remove it by
  backspacing. This seems to happen fairly regularly to me, so it seems
  worth checking for.
 
  I'm not sure where this code is. I'd want to see the context before
 deciding
  whether this is worth it.

 It occurs immediately prior to prompting the user whether we want to
 open the original or the emergency file. I consider it worthwhile
 since even IO is cheap compared to user time (and user concentration).

 This is now like:
 --
 Buffer::ReadStatus Buffer::readEmergency(FileName const  fn)
 {
   FileName const emergencyFile = getEmergencyFileNameFor(fn);
if (!emergencyFile.exists()
  || emergencyFile.lastModified() = fn.lastModified() ||
 +   emergencyFile.checksum() ==  fn.checksum())
return ReadFileNotFound;

docstring const file = makeDisplayPath(fn.absFileName(), 20);
 

 (and similar for autosave files)

  This is the code for the Recover All emergency save.
 
  Can you redo this, separately, given the recent changes?

 Sure (attached), but note that without having an e.g. Keep All
 option, if you open several files you'll still have several windows
 popping up asking you if you want to keep the emergency saves.

 --
 John C. McCabe-Dansted

Re: Improvements to Recover Emergency File prompts. (pseudo-patch with discussion)

[Richard Heck]

On 10/25/2010 01:23 PM, John McCabe-Dansted wrote:

On Tue, Oct 26, 2010 at 12:03 AM, Richard Heckrgh...@comcast.net  wrote:
   

I think this was added a while ago, because we were leaving emergency files
lying around. We only ask about this if an attempt to recover the file has
been made, either successfully or not. So either the document is now loaded
in the buffer or else we couldn't load it.
 

Again, I'd personally put this in ::save(). Leaving an emergency file
around until the user next saves doesn't sound too bad to me. If
everything goes well the user will continue working on the file and
save it soon enough. If it doesn't go well, keeping the file around is
probably a good idea. (The user might not know whether everything will
go well at the point we ask them)

   

Otherwise I'd add a Keep All option.
   

What does this mean?
 

Once you click Keep All, we keep all the saves without prompting the user.

   

+e.checksum() != s.checksum()) {

There are a number of ways to get a dirty buffer without actually
changing the file. E.g we can add two spaces that get converted to a
single space, or we can add a character and then remove it by
backspacing. This seems to happen fairly regularly to me, so it seems
worth checking for.
   

I'm not sure where this code is. I'd want to see the context before deciding
whether this is worth it.
 

It occurs immediately prior to prompting the user whether we want to
open the original or the emergency file. I consider it worthwhile
since even IO is cheap compared to user time (and user concentration).

   

OK. Does anyone else have a view about this?


This is now like:
--
Buffer::ReadStatus Buffer::readEmergency(FileName const  fn)
{
FileName const emergencyFile = getEmergencyFileNameFor(fn);
 if (!emergencyFile.exists()
   || emergencyFile.lastModified()= fn.lastModified() ||
+   emergencyFile.checksum() ==  fn.checksum())
 return ReadFileNotFound;

 docstring const file = makeDisplayPath(fn.absFileName(), 20);


(and similar for autosave files)

   

This is the code for the Recover All emergency save.
   

Can you redo this, separately, given the recent changes?
 

Sure (attached), but note that without having an e.g. Keep All
option, if you open several files you'll still have several windows
popping up asking you if you want to keep the emergency saves.

   
Unless I'm mistaken, recover_all remains true once set true. That is, it 
won't just effect the relatives of the file I'm opening now, but will 
also affect any file I open later. I.e., say I previously has a.lyx, 
b.lyx, and c.lyx open, where b is a child of a, but c is independent; 
LyX crashes. Now I restart LyX and open a.lyx and choose Recover all, 
so b.lyx gets opened from the emergency file. Now I go to open c.lyx, 
and it does, too, without my being asked.


We need some way to constrain this to relatives of the first file we try 
to open in this batch.


Richard

Re: Improvements to Recover Emergency File prompts. (pseudo-patch with discussion)

[Pavel Sanda]

John McCabe-Dansted wrote:
 - if (!Alert::prompt(_(Delete emergency file?), str, 1, 
 1,
 - _(Remove), _(Keep it))) {
 -  ...
 
 I think we should just keep the
 emergency file (unless it is obsolete). 

no, i dont agree here. it instantly drove me crazy that lyx was not able
the remove emergency files by itself so please let this dialog alive.

pavel

Re: Improvements to Recover Emergency File prompts. (pseudo-patch with discussion)

[Guenter Milde]

On 2010-10-25, Richard Heck wrote:
 On 10/25/2010 01:23 PM, John McCabe-Dansted wrote:
 On Tue, Oct 26, 2010 at 12:03 AM, Richard Heckrgh...@comcast.net  wrote:


 +e.checksum() != s.checksum()) {

 There are a number of ways to get a dirty buffer without actually
 changing the file. E.g we can add two spaces that get converted to a
 single space, or we can add a character and then remove it by
 backspacing. This seems to happen fairly regularly to me, so it seems
 worth checking for.

 I'm not sure where this code is. I'd want to see the context before deciding
 whether this is worth it.

 It occurs immediately prior to prompting the user whether we want to
 open the original or the emergency file. I consider it worthwhile
 since even IO is cheap compared to user time (and user concentration).

 OK. Does anyone else have a view about this?

I'd like this test also before the save buffer? popup when closing LyX.
(Currently, even change-something; undo will trigger this prompt.)

Günter

Re: Improvements to Recover Emergency File prompts. (pseudo-patch with discussion)

[Richard Heck]

On 10/25/2010 04:22 PM, Pavel Sanda wrote:

John McCabe-Dansted wrote:
   

-   if (!Alert::prompt(_(Delete emergency file?), str, 1, 
1,
-   _(Remove), _(Keep it))) {
-  ...

I think we should just keep the
emergency file (unless it is obsolete).
 

no, i dont agree here. it instantly drove me crazy that lyx was not able
the remove emergency files by itself so please let this dialog alive.

   

There was actually a bug report about this, if I remember correctly.

Richard

Re: Improvements to Recover Emergency File prompts. (pseudo-patch with discussion)

[Vincent van Ravesteijn]

 no, i dont agree here. it instantly drove me crazy that lyx was not able
 the remove emergency files by itself so please let this dialog alive.

 pavel


Well this dialog is just as frustrating :(.

Vincent

Re: Improvements to Recover Emergency File prompts. (pseudo-patch with discussion)

[Pavel Sanda]

Richard Heck wrote:
 -   if (!Alert::prompt(_(Delete emergency file?), str, 1, 
 1,
 -   _(Remove), _(Keep it))) {
 -  ...

 I think we should just keep the
 emergency file (unless it is obsolete).
  
 no, i dont agree here. it instantly drove me crazy that lyx was not able
 the remove emergency files by itself so please let this dialog alive.


 There was actually a bug report about this, if I remember correctly.

yes there was bug about it was and we fixed it by the dialog obove not so long 
ago.
constant re-asking again about recovering the file until one gets deep down in
dir structure where the file is located.

pavel

Re: Improvements to Recover Emergency File prompts. (pseudo-patch with discussion)

[John McCabe-Dansted]

On Tue, Oct 26, 2010 at 5:22 AM, Pavel Sanda sa...@lyx.org wrote:
 Richard Heck wrote:
 I think we should just keep the
 emergency file (unless it is obsolete).

 no, i dont agree here. it instantly drove me crazy that lyx was not able
 the remove emergency files by itself so please let this dialog alive.


 There was actually a bug report about this, if I remember correctly.

 yes there was bug about it was and we fixed it by the dialog obove not so 
 long ago.
 constant re-asking again about recovering the file until one gets deep down in
 dir structure where the file is located.

By constant re-asking, do you mean asking each time LyX starts?

Would a comment on the dialog saying To prevent this dialog being
displayed in future, please save this document suffice?

To me, deleting these files on Save seems the correct place; at the
point we can infer that the user has decided what they want to do. As
it is I just comment out the deletes in my tree because they have
burnt and/or frustrated me too often.

-- 
John C. McCabe-Dansted

Improvements to "Recover Emergency File" prompts. (pseudo-patch with discussion)

[John McCabe-Dansted]

I've been using a number of changes to Buffer::readFileHelper that
I've found useful. I err on the side off not delete emergency files,
and also add a Recover All option that is useful when you have a
master document that has several modified child documents.

I think that these modifications require a little discussion before
being formally proposed, so I discuss them inline rather than submit
them as a formal patch.

--
switch (Alert::prompt(_("Load backup?"), text, 0, 2,
  _(" backup"), _("Load "),
  _("") ))
{
...
case 1:
-   // Here we delete the autosave
-   a.removeFile();

I don't see why we delete the autosave here. What I sometimes do is think:
   "OK, I was doing some experimental stuff. I should probably open
the original file. Opps, I didn't do a proper save in the last hour,
never-mind I'll just restart LyX and pick load Backup. Huh, my
autosave file and all my work is gone!"

Simply removing that line prevents that problem, and doesn't seem to
cause any significant regressions.

If we want to automatically delete the autosave file, I'd do it once
we have verified that the file is old and obsolete. E.g. something
like the psuedo-code:

IF auto-save is newer
Prompt user etc.
ELSE
delete auto-save.

-

-   if (!Alert::prompt(_("Delete emergency file?"), str, 1, 
1,
-   _(""), _(" it"))) {
-  ...

Well, here at least we warn the user that we are about to delete
stuff. However, when LyX crashes the only thing on my mind is getting
my data back as quickly as possible. I think we should just keep the
emergency file (unless it is obsolete). We could possibly ask the user
this question on shutdown.

Otherwise I'd add a "Keep All" option.

-
-   if (e.exists() && s.exists() && e.lastModified() > s.lastModified()) {
+
+   if (e.exists() && s.exists() && e.lastModified() > s.lastModified() &&
+e.checksum() != s.checksum()) {

There are a number of ways to get a dirty buffer without actually
changing the file. E.g we can add two spaces that get converted to a
single space, or we can add a character and then remove it by
backspacing. This seems to happen fairly regularly to me, so it seems
worth checking for.

However I am not sure .checksum() is the best way to do it. The
checksum is a long, so even if we assume the checksum algorithm is
ideal, we still get a false positive once per 4 billion checks. It
seems like something like:
e.fileContents() == s.fileContents()
would be cleaner and just as fast. However that could use a lot of
memory on large files. It seems the best way would be to do:
e.fileContentsEqual(s)
where FileName::fileContentsEqual is a new method that compares the
two files in (say) 1MB chunks. However I think it would be adequate to
implement this as something like:

bool FileName::fileContentsEqual(FileName const & other_file)
{
return (d->fi.size() == other_file->fi.size() &&
fileContents() == other_file->fileContents())
}

as
0) this is way simpler,
1)  it seems unlikely that we would have a single valid lyx file that
does not easily fit in memory,
2)  the file size check should stop us from loading emergency files
larger than the valid lyx file, and
3) we could always improve fileContentsEqual later if need be.

-
-   switch (Alert::prompt(_("Load emergency save?"), text, 0, 2,
- _(""),  _(" Original"),
- _("")))
+   int ret;
+   if (recover_all) {
+   ret = 0;
+   } else {
+   ret = Alert::prompt(_("Load emergency save?"),
+   text, 0, 2,
+   _(""),  _(" Original"),
+   _(""), _("Recover "));
+   if (ret == 3) {
+   ret = 0;
+   recover_all = true;
+   }
+   }
+   switch (ret)

This is the code for the "Recover All" emergency save.

-- 
John C. McCabe-Dansted

Re: Improvements to "Recover Emergency File" prompts. (pseudo-patch with discussion)

[Richard Heck]

On 10/25/2010 03:58 AM, John McCabe-Dansted wrote:

-

-   if (!Alert::prompt(_("Delete emergency file?"), str, 1, 
1,
-   _(""), _(" it"))) {
-  ...

Well, here at least we warn the user that we are about to delete
stuff. However, when LyX crashes the only thing on my mind is getting
my data back as quickly as possible. I think we should just keep the
emergency file (unless it is obsolete). We could possibly ask the user
this question on shutdown.

   
I think this was added a while ago, because we were leaving emergency 
files lying around. We only ask about this if an attempt to recover the 
file has been made, either successfully or not. So either the document 
is now loaded in the buffer or else we couldn't load it.



Otherwise I'd add a "Keep All" option.

   

What does this mean?


-
-   if (e.exists()&&  s.exists()&&  e.lastModified()>  s.lastModified()) {
+
+   if (e.exists()&&  s.exists()&&  e.lastModified()>  s.lastModified()&&
+e.checksum() != s.checksum()) {

There are a number of ways to get a dirty buffer without actually
changing the file. E.g we can add two spaces that get converted to a
single space, or we can add a character and then remove it by
backspacing. This seems to happen fairly regularly to me, so it seems
worth checking for.

   
I'm not sure where this code is. I'd want to see the context before 
deciding whether this is worth it.



However I am not sure .checksum() is the best way to do it. The
checksum is a long, so even if we assume the checksum algorithm is
ideal, we still get a false positive once per 4 billion checks. It
seems like something like:
 e.fileContents() == s.fileContents()
would be cleaner and just as fast.

   
Just use some new function equalContents(e,s). We don't actually care 
about the contents; only if they're equal. Then you can use an 
istreambuf_iterator, as in the checksum code, and the memory issue is 
less serious. You can also return false as soon as you find a difference.



-
-   switch (Alert::prompt(_("Load emergency save?"), text, 0, 2,
- _(""),  _(" Original"),
- _("")))
+   int ret;
+   if (recover_all) {
+   ret = 0;
+   } else {
+   ret = Alert::prompt(_("Load emergency save?"),
+   text, 0, 2,
+   _(""),  _(" Original"),
+   _(""), _("Recover"));
+   if (ret == 3) {
+   ret = 0;
+   recover_all = true;
+   }
+   }
+   switch (ret)

This is the code for the "Recover All" emergency save.

   

Can you redo this, separately, given the recent changes?

Richard

Re: Improvements to "Recover Emergency File" prompts. (pseudo-patch with discussion)

[John McCabe-Dansted]

On Tue, Oct 26, 2010 at 12:03 AM, Richard Heck  wrote:
> I think this was added a while ago, because we were leaving emergency files
> lying around. We only ask about this if an attempt to recover the file has
> been made, either successfully or not. So either the document is now loaded
> in the buffer or else we couldn't load it.

Again, I'd personally put this in ::save(). Leaving an emergency file
around until the user next saves doesn't sound too bad to me. If
everything goes well the user will continue working on the file and
save it soon enough. If it doesn't go well, keeping the file around is
probably a good idea. (The user might not know whether everything will
go well at the point we ask them)

>> Otherwise I'd add a "Keep All" option.
>
> What does this mean?

Once you click "Keep All", we keep all the saves without prompting the user.

>> +                        e.checksum() != s.checksum()) {
>>
>> There are a number of ways to get a dirty buffer without actually
>> changing the file. E.g we can add two spaces that get converted to a
>> single space, or we can add a character and then remove it by
>> backspacing. This seems to happen fairly regularly to me, so it seems
>> worth checking for.
>
> I'm not sure where this code is. I'd want to see the context before deciding
> whether this is worth it.

It occurs immediately prior to prompting the user whether we want to
open the original or the emergency file. I consider it worthwhile
since even IO is cheap compared to user time (and user concentration).

This is now like:
--
Buffer::ReadStatus Buffer::readEmergency(FileName const & fn)
{
   FileName const emergencyFile = getEmergencyFileNameFor(fn);
if (!emergencyFile.exists()
  || emergencyFile.lastModified() <= fn.lastModified() ||
+   emergencyFile.checksum() ==  fn.checksum())
return ReadFileNotFound;

docstring const file = makeDisplayPath(fn.absFileName(), 20);


(and similar for autosave files)

>> This is the code for the "Recover All" emergency save.
>
> Can you redo this, separately, given the recent changes?

Sure (attached), but note that without having an e.g. "Keep All"
option, if you open several files you'll still have several windows
popping up asking you if you want to keep the emergency saves.

-- 
John C. McCabe-Dansted
Index: Buffer.cpp
===
--- Buffer.cpp	(revision 35834)
+++ Buffer.cpp	(working copy)
@@ -3617,6 +3627,8 @@
 
 Buffer::ReadStatus Buffer::readEmergency(FileName const & fn)
 {
+	static bool recover_all = false;
+
 	FileName const emergencyFile = getEmergencyFileNameFor(fn);
 	if (!emergencyFile.exists() 
 		  || emergencyFile.lastModified() <= fn.lastModified())
@@ -3626,9 +3638,20 @@
 	docstring const text = bformat(_("An emergency save of the document "
 		"%1$s exists.\n\nRecover emergency save?"), file);
 	
-	int const load_emerg = Alert::prompt(_("Load emergency save?"), text,
-		0, 2, _(""), _(" Original"), _(""));
+	int load_emerg;
 
+	if (recover_all) {
+		load_emerg = 0;
+	} else {
+		load_emerg = Alert::prompt(_("Load emergency save?"), text,
+			0, 2, _(""), _(" Original"),
+			_(""), _("Recover "));
+		if (load_emerg == 3) {
+			load_emerg = 0;
+			recover_all = true;
+		}
+	}
+
 	switch (load_emerg)
 	{
 	case 0: {

Re: Improvements to "Recover Emergency File" prompts. (pseudo-patch with discussion)

[Jose Quesada]

Having lost a few mins of inspired writing, I'm all for leaving the
emergency files lying around...
Best,
-Jose

Jose Quesada, PhD.
Research scientist,
Max Planck Institute,
Center for Adaptive Behavior and Cognition,
Berlin
http://www.josequesada.name/
http://twitter.com/Quesada


On Mon, Oct 25, 2010 at 7:23 PM, John McCabe-Dansted wrote:

> On Tue, Oct 26, 2010 at 12:03 AM, Richard Heck  wrote:
> > I think this was added a while ago, because we were leaving emergency
> files
> > lying around. We only ask about this if an attempt to recover the file
> has
> > been made, either successfully or not. So either the document is now
> loaded
> > in the buffer or else we couldn't load it.
>
> Again, I'd personally put this in ::save(). Leaving an emergency file
> around until the user next saves doesn't sound too bad to me. If
> everything goes well the user will continue working on the file and
> save it soon enough. If it doesn't go well, keeping the file around is
> probably a good idea. (The user might not know whether everything will
> go well at the point we ask them)
>
> >> Otherwise I'd add a "Keep All" option.
> >
> > What does this mean?
>
> Once you click "Keep All", we keep all the saves without prompting the
> user.
>
> >> +e.checksum() != s.checksum()) {
> >>
> >> There are a number of ways to get a dirty buffer without actually
> >> changing the file. E.g we can add two spaces that get converted to a
> >> single space, or we can add a character and then remove it by
> >> backspacing. This seems to happen fairly regularly to me, so it seems
> >> worth checking for.
> >
> > I'm not sure where this code is. I'd want to see the context before
> deciding
> > whether this is worth it.
>
> It occurs immediately prior to prompting the user whether we want to
> open the original or the emergency file. I consider it worthwhile
> since even IO is cheap compared to user time (and user concentration).
>
> This is now like:
> --
> Buffer::ReadStatus Buffer::readEmergency(FileName const & fn)
> {
>   FileName const emergencyFile = getEmergencyFileNameFor(fn);
>if (!emergencyFile.exists()
>  || emergencyFile.lastModified() <= fn.lastModified() ||
> +   emergencyFile.checksum() ==  fn.checksum())
>return ReadFileNotFound;
>
>docstring const file = makeDisplayPath(fn.absFileName(), 20);
> 
>
> (and similar for autosave files)
>
> >> This is the code for the "Recover All" emergency save.
> >
> > Can you redo this, separately, given the recent changes?
>
> Sure (attached), but note that without having an e.g. "Keep All"
> option, if you open several files you'll still have several windows
> popping up asking you if you want to keep the emergency saves.
>
> --
> John C. McCabe-Dansted
>

Re: Improvements to "Recover Emergency File" prompts. (pseudo-patch with discussion)

[Richard Heck]

On 10/25/2010 01:23 PM, John McCabe-Dansted wrote:

On Tue, Oct 26, 2010 at 12:03 AM, Richard Heck  wrote:
   

I think this was added a while ago, because we were leaving emergency files
lying around. We only ask about this if an attempt to recover the file has
been made, either successfully or not. So either the document is now loaded
in the buffer or else we couldn't load it.
 

Again, I'd personally put this in ::save(). Leaving an emergency file
around until the user next saves doesn't sound too bad to me. If
everything goes well the user will continue working on the file and
save it soon enough. If it doesn't go well, keeping the file around is
probably a good idea. (The user might not know whether everything will
go well at the point we ask them)

   

Otherwise I'd add a "Keep All" option.
   

What does this mean?
 

Once you click "Keep All", we keep all the saves without prompting the user.

   

+e.checksum() != s.checksum()) {

There are a number of ways to get a dirty buffer without actually
changing the file. E.g we can add two spaces that get converted to a
single space, or we can add a character and then remove it by
backspacing. This seems to happen fairly regularly to me, so it seems
worth checking for.
   

I'm not sure where this code is. I'd want to see the context before deciding
whether this is worth it.
 

It occurs immediately prior to prompting the user whether we want to
open the original or the emergency file. I consider it worthwhile
since even IO is cheap compared to user time (and user concentration).

   

OK. Does anyone else have a view about this?


This is now like:
--
Buffer::ReadStatus Buffer::readEmergency(FileName const&  fn)
{
FileName const emergencyFile = getEmergencyFileNameFor(fn);
 if (!emergencyFile.exists()
   || emergencyFile.lastModified()<= fn.lastModified() ||
+   emergencyFile.checksum() ==  fn.checksum())
 return ReadFileNotFound;

 docstring const file = makeDisplayPath(fn.absFileName(), 20);


(and similar for autosave files)

   

This is the code for the "Recover All" emergency save.
   

Can you redo this, separately, given the recent changes?
 

Sure (attached), but note that without having an e.g. "Keep All"
option, if you open several files you'll still have several windows
popping up asking you if you want to keep the emergency saves.

   
Unless I'm mistaken, recover_all remains true once set true. That is, it 
won't just effect the relatives of the file I'm opening now, but will 
also affect any file I open later. I.e., say I previously has a.lyx, 
b.lyx, and c.lyx open, where b is a child of a, but c is independent; 
LyX crashes. Now I restart LyX and open a.lyx and choose "Recover all", 
so b.lyx gets opened from the emergency file. Now I go to open c.lyx, 
and it does, too, without my being asked.


We need some way to constrain this to relatives of the first file we try 
to open in this "batch".


Richard

Re: Improvements to "Recover Emergency File" prompts. (pseudo-patch with discussion)

[Pavel Sanda]

John McCabe-Dansted wrote:
> - if (!Alert::prompt(_("Delete emergency file?"), str, 1, 
> 1,
> - _(""), _(" it"))) {
> -  ...
> 
> I think we should just keep the
> emergency file (unless it is obsolete). 

no, i dont agree here. it instantly drove me crazy that lyx was not able
the remove emergency files by itself so please let this dialog alive.

pavel

Re: Improvements to "Recover Emergency File" prompts. (pseudo-patch with discussion)

[Guenter Milde]

On 2010-10-25, Richard Heck wrote:
> On 10/25/2010 01:23 PM, John McCabe-Dansted wrote:
>> On Tue, Oct 26, 2010 at 12:03 AM, Richard Heck  wrote:


 +e.checksum() != s.checksum()) {

 There are a number of ways to get a dirty buffer without actually
 changing the file. E.g we can add two spaces that get converted to a
 single space, or we can add a character and then remove it by
 backspacing. This seems to happen fairly regularly to me, so it seems
 worth checking for.

>>> I'm not sure where this code is. I'd want to see the context before deciding
>>> whether this is worth it.

>> It occurs immediately prior to prompting the user whether we want to
>> open the original or the emergency file. I consider it worthwhile
>> since even IO is cheap compared to user time (and user concentration).

> OK. Does anyone else have a view about this?

I'd like this test also before the "save buffer?" popup when closing LyX.
(Currently, even "; undo" will trigger this prompt.)

Günter

Re: Improvements to "Recover Emergency File" prompts. (pseudo-patch with discussion)

[Richard Heck]

On 10/25/2010 04:22 PM, Pavel Sanda wrote:

John McCabe-Dansted wrote:
   

-   if (!Alert::prompt(_("Delete emergency file?"), str, 1, 
1,
-   _(""), _(" it"))) {
-  ...

I think we should just keep the
emergency file (unless it is obsolete).
 

no, i dont agree here. it instantly drove me crazy that lyx was not able
the remove emergency files by itself so please let this dialog alive.

   

There was actually a bug report about this, if I remember correctly.

Richard

Re: Improvements to "Recover Emergency File" prompts. (pseudo-patch with discussion)

[Vincent van Ravesteijn]

> no, i dont agree here. it instantly drove me crazy that lyx was not able
> the remove emergency files by itself so please let this dialog alive.
>
> pavel
>

Well this dialog is just as frustrating :(.

Vincent

Re: Improvements to "Recover Emergency File" prompts. (pseudo-patch with discussion)

[Pavel Sanda]

Richard Heck wrote:
>>> -   if (!Alert::prompt(_("Delete emergency file?"), str, 1, 
>>> 1,
>>> -   _(""), _(" it"))) {
>>> -  ...
>>>
>>> I think we should just keep the
>>> emergency file (unless it is obsolete).
>>>  
>> no, i dont agree here. it instantly drove me crazy that lyx was not able
>> the remove emergency files by itself so please let this dialog alive.
>>
>>
> There was actually a bug report about this, if I remember correctly.

yes there was bug about it was and we fixed it by the dialog obove not so long 
ago.
constant re-asking again about recovering the file until one gets deep down in
dir structure where the file is located.

pavel

Re: Improvements to "Recover Emergency File" prompts. (pseudo-patch with discussion)

[John McCabe-Dansted]

On Tue, Oct 26, 2010 at 5:22 AM, Pavel Sanda  wrote:
> Richard Heck wrote:
 I think we should just keep the
 emergency file (unless it is obsolete).

>>> no, i dont agree here. it instantly drove me crazy that lyx was not able
>>> the remove emergency files by itself so please let this dialog alive.
>>>
>>>
>> There was actually a bug report about this, if I remember correctly.
>
> yes there was bug about it was and we fixed it by the dialog obove not so 
> long ago.
> constant re-asking again about recovering the file until one gets deep down in
> dir structure where the file is located.

By constant re-asking, do you mean asking each time LyX starts?

Would a comment on the dialog saying "To prevent this dialog being
displayed in future, please save this document" suffice?

To me, deleting these files on Save seems the correct place; at the
point we can infer that the user has decided what they want to do. As
it is I just comment out the deletes in my tree because they have
burnt and/or frustrated me too often.

-- 
John C. McCabe-Dansted