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

Reply via email to