JAL-3141 Ostensibly all backups working for alignment and project saves. Need to...
authorBen Soares <bsoares@dundee.ac.uk>
Thu, 25 Oct 2018 15:36:41 +0000 (16:36 +0100)
committerBen Soares <bsoares@dundee.ac.uk>
Thu, 25 Oct 2018 15:36:41 +0000 (16:36 +0100)
resources/lang/Messages.properties
resources/lang/Messages_es.properties
src/jalview/gui/AlignFrame.java
src/jalview/gui/Jalview2XML.java
src/jalview/io/BackupFilenameFilter.java
src/jalview/io/BackupFiles.java
src/jalview/io/JalviewFileChooser.java

index ae5b0e7..bcd63ae 100644 (file)
@@ -1363,3 +1363,7 @@ label.most_bound_molecules = Most Bound Molecules
 label.most_polymer_residues = Most Polymer Residues
 label.cached_structures = Cached Structures
 label.free_text_search = Free Text Search
+label.backupfiles_confirm_delete = Confirm delete
+label.backupfiles_confirm_delete_old_files = Delete the following older version files?
+label.backupfiles_confirm_save_file = Confirm save file
+label.backupfiles_confirm_save_file_backupfiles_roll_wrong = Something possibly went wrong with the backups of this file, write the new file anyway?
index 555977d..9498aaf 100644 (file)
@@ -1364,3 +1364,7 @@ label.most_bound_molecules = M
 label.most_polymer_residues = Más Residuos de Polímeros
 label.cached_structures = Estructuras en Caché
 label.free_text_search = Búsqueda de texto libre
+label.backupfiles_confirm_delete = Confirmar borrar
+label.backupfiles_confirm_delete_old_files = ¿Borrar los siguientes archivos?
+label.backupfiles_confirm_save_file = Confirmar guardar archivo
+label.backupfiles_confirm_save_file_backupfiles_roll_wrong = Posiblemente algo está mal con los archivos de copia de seguridad. ¿Guardar el nuevo archivo?
index 94b38ed..268139d 100644 (file)
@@ -64,6 +64,7 @@ import jalview.gui.ColourMenuHelper.ColourChangeListener;
 import jalview.gui.ViewSelectionMenu.ViewSetProvider;
 import jalview.io.AlignmentProperties;
 import jalview.io.AnnotationFile;
+import jalview.io.BackupFiles;
 import jalview.io.BioJsHTMLOutput;
 import jalview.io.DataSourceType;
 import jalview.io.FileFormat;
@@ -1186,9 +1187,13 @@ public class AlignFrame extends GAlignFrame implements DropTargetListener,
       }
       else
       {
+        // create backupfiles object and get new temp filename destination
+        BackupFiles backupfiles = new BackupFiles(file);
+
         try
         {
-          PrintWriter out = new PrintWriter(new FileWriter(file));
+          PrintWriter out = new PrintWriter(
+                  new FileWriter(backupfiles.getTempFilePath()));
 
           out.print(output);
           out.close();
@@ -1201,6 +1206,11 @@ public class AlignFrame extends GAlignFrame implements DropTargetListener,
           success = false;
           ex.printStackTrace();
         }
+
+        backupfiles.setWriteSuccess(success);
+        // do the backup file roll and rename the temp file to actual file
+        success = backupfiles.rollBackupsAndRenameTempFile();
+
       }
     }
 
index 9285754..2c3d2d2 100644 (file)
@@ -43,6 +43,7 @@ import jalview.datamodel.features.FeatureMatcherSet;
 import jalview.datamodel.features.FeatureMatcherSetI;
 import jalview.ext.varna.RnaModel;
 import jalview.gui.StructureViewer.ViewerType;
+import jalview.io.BackupFiles;
 import jalview.io.DataSourceType;
 import jalview.io.FileFormat;
 import jalview.renderer.ResidueShaderI;
@@ -52,7 +53,6 @@ import jalview.schemabinding.version2.Annotation;
 import jalview.schemabinding.version2.AnnotationColours;
 import jalview.schemabinding.version2.AnnotationElement;
 import jalview.schemabinding.version2.CalcIdParam;
-import jalview.schemabinding.version2.Colour;
 import jalview.schemabinding.version2.CompoundMatcher;
 import jalview.schemabinding.version2.DBRef;
 import jalview.schemabinding.version2.Features;
@@ -463,10 +463,16 @@ public class Jalview2XML
     FileOutputStream fos = null;
     try
     {
-      fos = new FileOutputStream(statefile);
+
+      BackupFiles backupfiles = new BackupFiles(statefile);
+      fos = new FileOutputStream(backupfiles.getTempFile());
+
       JarOutputStream jout = new JarOutputStream(fos);
       saveState(jout);
 
+      backupfiles.setWriteSuccess(true);
+      backupfiles.rollBackupsAndRenameTempFile();
+
     } catch (Exception e)
     {
       // TODO: inform user of the problem - they need to know if their data was
@@ -647,7 +653,11 @@ public class Jalview2XML
   {
     try
     {
-      FileOutputStream fos = new FileOutputStream(jarFile);
+      // create backupfiles object and get new temp filename destination
+      BackupFiles backupfiles = new BackupFiles(jarFile);
+      FileOutputStream fos = new FileOutputStream(
+              backupfiles.getTempFilePath());
+
       JarOutputStream jout = new JarOutputStream(fos);
       List<AlignFrame> frames = new ArrayList<>();
 
@@ -669,7 +679,12 @@ public class Jalview2XML
       }
       ;
       jout.close();
-      return true;
+      boolean success = true;
+
+      backupfiles.setWriteSuccess(success);
+      success = backupfiles.rollBackupsAndRenameTempFile();
+
+      return success;
     } catch (Exception ex)
     {
       errorMessage = "Couldn't Write alignment view to Jalview Archive - see error output for details";
index 8cf7cbf..568f1c9 100644 (file)
@@ -2,6 +2,7 @@ package jalview.io;
 
 import java.io.File;
 import java.io.FilenameFilter;
+import java.io.IOException;
 
 public class BackupFilenameFilter implements FilenameFilter
 {
@@ -24,27 +25,34 @@ public class BackupFilenameFilter implements FilenameFilter
   }
 
   @Override
-  public boolean accept(File file, String filename)
+  public boolean accept(File dir, String filename)
   {
-    // CHECK THIS IS NOT ALWAYS THE PARENT DIR
-    if (file.isDirectory())
+    boolean ret = false;
+    try
     {
-      return true;
-    }
-    else
+      File file = new File(
+              dir.getCanonicalPath() + File.separatorChar + filename);
+      if (file.isDirectory())
+      {
+        // backup files aren't dirs!
+        return false;
+      }
+    } catch (IOException e)
     {
-      BackupFilenameParts bffp = new BackupFilenameParts(filename, base,
-              template, digits, extension);
-      return bffp.isBackupFile();
+      System.out.println("IOException when checking file '" + filename
+              + "' is a backupfile");
     }
+
+    BackupFilenameParts bffp = new BackupFilenameParts(filename, base,
+            template, digits, extension);
+    ret = bffp.isBackupFile();
+    return ret;
   }
 
 }
 
 class BackupFilenameParts
 {
-  File file;
-
   String base;
 
   String templateStart;
@@ -70,8 +78,15 @@ class BackupFilenameParts
   {
     this.isBackupFile = false;
 
-    if (!(filename.startsWith(base) && filename.endsWith(extension)))
+    // calculate minimum length of a backup filename
+    int minlength = base.length() + template.length()
+            - BackupFiles.NUM_PLACEHOLDER.length() + digits
+            + extension.length();
+
+    if (!(filename.startsWith(base) && filename.endsWith(extension)
+            && filename.length() >= minlength))
     {
+      // non-starter
       return;
     }
 
@@ -83,16 +98,17 @@ class BackupFilenameParts
       templateStart = template.substring(0, numcharstart);
       templateEnd = template.substring(numcharstart + BackupFiles.NUM_PLACEHOLDER.length());
     }
+    
     int startLength = base.length() + templateStart.length();
     int endLength = templateEnd.length() + extension.length();
-    String numString = filename.substring(startLength, filename.length() - endLength + 1);
+    String numString = filename.substring(startLength,
+            filename.length() - endLength);
 
     if (filename.length() >= startLength + digits + endLength
             && filename.startsWith(base + templateStart)
             && filename.endsWith(templateEnd + extension)
             && numString.matches("[0-9]+"))
     {
-      this.file = file;
       this.base = base;
       this.templateStart = templateStart;
       this.num = Integer.parseInt(numString);
index 39cd23e..ff23cbf 100644 (file)
@@ -1,6 +1,9 @@
 package jalview.io;
 
 import jalview.bin.Cache;
+import jalview.gui.Desktop;
+import jalview.gui.JvOptionPane;
+import jalview.util.MessageManager;
 
 import java.io.File;
 import java.io.IOException;
@@ -16,6 +19,7 @@ import java.util.TreeMap;
  * BACKUPFILES_ROLL_MAX - the maximum number of backupfiles to keep for any one alignment or project file.
  * BACKUPFILES_SUFFIX_DIGITS - the number of digits to insert replace %n with (e.g. BACKUPFILES_SUFFIX_DIGITS = 3 would make "001", "002", etc)
  * BACKUPFILES_REVERSE_ORDER - if true then "logfile" style numbering and file rolling will occur. If false then ever-increasing version numbering will occur, but old files will still be deleted if there are more than ROLL_MAX backup files. 
+ * BACKUPFILES_CONFIRM_DELETE_OLD - if true then prompt/confirm with the user when deleting older backup/version files.
  */
 
 public class BackupFiles
@@ -36,6 +40,8 @@ public class BackupFiles
 
   public static String REVERSE_ORDER = NS + "_REVERSE_ORDER";
 
+  public static String CONFIRM_DELETE_OLD = NS + "_CONFIRM_DELETE_OLD";
+
   private static String DEFAULT_TEMP_FILE = "jalview_temp_file_" + NS;
 
   // file - File object to be backed up and then updated (written over)
@@ -43,7 +49,13 @@ public class BackupFiles
 
   // enabled - default flag as to whether to do the backup file roll (if not
   // defined in preferences)
-  private boolean enabled;
+  private static boolean enabled;
+
+  // confirmDelete - default flag as to whether to confirm with the user before
+  // deleting old backup/version files
+  private static boolean confirmDelete;
+
+  private static boolean classInit = false;
 
   // defaultSuffix - default template to use to append to basename of file
   private String suffix;
@@ -61,48 +73,34 @@ public class BackupFiles
   // temp saved file to become new saved file
   private File tempFile;
 
+  // flag set to see if file save to temp file was successful
+  private boolean tempFileWriteSuccess;
+
   public BackupFiles(String filename)
   {
     this(new File(filename));
   }
 
-  // first time defaults for ENABLED, SUFFIX, ROLL_MAX, SUFFIX_DIGITS and
-  // REVERSE_ORDER
+  // first time defaults for SUFFIX, ROLL_MAX, SUFFIX_DIGITS and REVERSE_ORDER
   public BackupFiles(File file)
   {
-    this(file, true, "-v" + NUM_PLACEHOLDER, 4, 3, false);
-  }
-
-  // set, get and rename temp file into place
-  public void setTempFile(File temp)
-  {
-    this.tempFile = temp;
-  }
-
-  public File getTempFile()
-  {
-    return tempFile;
-  }
-
-  public boolean renameTempFile()
-  {
-    return tempFile.renameTo(file);
+    this(file, "-v" + NUM_PLACEHOLDER, 4, 3, false);
   }
 
-  public BackupFiles(File file, boolean defaultEnabled,
-          String defaultSuffix,
-          int defaultMax, int defaultDigits, boolean defaultReverseOrder)
+  public BackupFiles(File file,
+          String defaultSuffix, int defaultMax, int defaultDigits,
+          boolean defaultReverseOrder)
   {
+    classInit();
     this.file = file;
-    this.enabled = Cache.getDefault(ENABLED, defaultEnabled);
     this.suffix = Cache.getDefault(SUFFIX, defaultSuffix);
     this.max = Cache.getDefault(ROLL_MAX, defaultMax);
     this.digits = Cache.getDefault(SUFFIX_DIGITS, defaultDigits);
     this.reverseOrder = Cache.getDefault(REVERSE_ORDER,
             defaultReverseOrder);
-    
+
     // create a temp file to save new data in
-    File temp;
+    File temp = null;
     try
     {
       if (file != null)
@@ -114,7 +112,6 @@ public class BackupFiles
       else
       {
         temp = File.createTempFile(DEFAULT_TEMP_FILE, ".tmp");
-        setTempFile(temp);
       }
     } catch (IOException e)
     {
@@ -124,15 +121,91 @@ public class BackupFiles
     {
       System.out.println("Exception ctreating temp file for saving");
     }
+    this.setTempFile(temp);
+  }
+
+  public static void classInit()
+  {
+    if (!classInit)
+    {
+      setEnabled(Cache.getDefault(ENABLED, true));
+      setConfirmDelete(Cache.getDefault(CONFIRM_DELETE_OLD, true));
+      classInit = true;
+    }
+  }
+
+  public static void setEnabled(boolean flag)
+  {
+    enabled = flag;
+  }
+
+  public static boolean getEnabled()
+  {
+    classInit();
+    return enabled;
+  }
+
+  public static void setConfirmDelete(boolean flag)
+  {
+    confirmDelete = flag;
+  }
+
+  public static boolean getConfirmDelete()
+  {
+    classInit();
+    return confirmDelete;
+  }
+
+  // set, get and rename temp file into place
+  public void setTempFile(File temp)
+  {
+    this.tempFile = temp;
+  }
+
+  public File getTempFile()
+  {
+    return tempFile;
+  }
+
+  public String getTempFilePath()
+  {
+    String path = null;
+    try
+    {
+      path = this.getTempFile().getCanonicalPath();
+    } catch (IOException e)
+    {
+      System.out.println(
+              "IOException when getting Canonical Path of temp file '"
+                      + this.getTempFile().getName() + "'");
+    }
+    return path;
+  }
 
+  public boolean setWriteSuccess(boolean flag)
+  {
+    boolean old = this.tempFileWriteSuccess;
+    this.tempFileWriteSuccess = flag;
+    return old;
+  }
+
+  public boolean getWriteSuccess()
+  {
+    return this.tempFileWriteSuccess;
+  }
+
+  public boolean renameTempFile()
+  {
+    return tempFile.renameTo(file);
   }
 
+
   // roll the backupfiles
   public boolean rollBackupFiles()
   {
 
     // file doesn't yet exist or backups are not enabled
-    if ((!file.exists()) || (!enabled) || (max < 1))
+    if ((!file.exists()) || (!enabled) || (max < -1))
     {
       // nothing to do
       return true;
@@ -166,128 +239,223 @@ public class BackupFiles
 
     boolean ret = true;
     // Create/move backups up one
-    String numString = null;
-    File lastfile = null;
 
-    if (reverseOrder)
+    File[] oldFilesToDelete = null;
+    
+    // find existing backup files
+    BackupFilenameFilter bff = new BackupFilenameFilter(basename, suffix,
+            digits,
+            extension);
+    File[] backupFiles = dirFile.listFiles(bff);
+    int nextIndexNum = 0;
+    String confirmDeleteExtraInfo = null;
+    
+    if (backupFiles.length == 0)
+    {
+      // No other backup files. Just need to move existing file to backupfile_1
+      nextIndexNum = 1;
+    }
+    else
     {
-      // backup style numbering
 
-      int tempMax = max;
-      // max == -1 means no limits
-      if (max == -1)
+      // sort the backup files (based on integer found in the suffix) using a
+      // precomputed Hashmap for speed
+      HashMap<Integer, File> bfHashMap = new HashMap<>();
+      for (int i = 0; i < backupFiles.length; i++)
       {
-        // do something cleverer here (possibly)!
-        tempMax = 10000;
+        File f = backupFiles[i];
+        BackupFilenameParts bfp = new BackupFilenameParts(f, basename, suffix, digits, extension);
+        bfHashMap.put(bfp.indexNum(), f);
       }
+      TreeMap<Integer, File> bfTreeMap = new TreeMap<>();
+      bfTreeMap.putAll(bfHashMap);
 
-      for (int m = 0; m < tempMax; m++)
+      if (reverseOrder)
       {
-        int n = tempMax - m;
-        String backupfilename = dir + File.separatorChar
-                + BackupFilenameParts.getBackupFilename(n, basename, suffix,
-                        digits, extension);
-        File backupfile_n = new File(backupfilename);
-
-        if (!backupfile_n.exists())
+        // backup style numbering
+
+        File lastfile = null;
+        int tempMax = max;
+        // max == -1 means no limits
+        // look for first "gap" in backupFiles
+        // if tempMax is -1 at this stage just keep going until there's a gap...
+        // why do I feel a little uneasy about this loop?..
+        for (int i = 1; tempMax < 0 || (max >= 0 && i <= max); i++)
         {
-          lastfile = backupfile_n;
-          continue;
+          if (!bfTreeMap.containsKey(i)) // first non-existent backupfile
+          {
+            tempMax = i;
+          }
         }
 
-        if (m == 0)
-        { // Move the max backup to /tmp instead of deleting (Just In
-          // Case)
-          String tmpfile = "tmp-" + backupfile_n.getName();
-          try
+        for (int m = 0; m < tempMax; m++)
+        {
+          int n = tempMax - m;
+          String backupfilename = dir + File.separatorChar
+                  + BackupFilenameParts.getBackupFilename(n, basename,
+                          suffix, digits, extension);
+          File backupfile_n = new File(backupfilename);
+
+          if (!backupfile_n.exists())
           {
-            File tmpFile = File.createTempFile(tmpfile, ".tmp");
-            ret = ret && backupfile_n.renameTo(tmpFile);
-          } catch (IOException e)
+            lastfile = backupfile_n;
+            continue;
+          }
+
+          if (m == 0 && backupfile_n.exists())
           {
-            System.out.println(
-                    "Could not create temp file '" + tmpfile + ".tmp'");
+            // move the largest (max) rolled file to a temp file and add to the delete list
+            try
+            {
+              File temp = File.createTempFile(backupfilename, ".tmp",
+                    dirFile);
+              backupfile_n.renameTo(temp);
+
+              oldFilesToDelete = new File[] { temp };
+              confirmDeleteExtraInfo = "(was " + backupfile_n.getName()
+                      + ")";
+            } catch (IOException e)
+            {
+              System.out.println(
+                      "IOException when creating temporary file for backupfilename");
+            }
           }
-        }
-        else
-        {
-          // Just In Case
-          if (lastfile != null)
+          else
           {
-            ret = ret && backupfile_n.renameTo(lastfile);
+            // Just In Case
+            if (lastfile != null)
+            {
+              ret = ret && backupfile_n.renameTo(lastfile);
+            }
           }
+
+          lastfile = backupfile_n;
         }
 
-        lastfile = backupfile_n;
+        // index to use for the latest backup
+        nextIndexNum = 1;
       }
-
-      // now actually backup the important file!
-      ret = ret && file.renameTo(lastfile);
-    }
-    else
-    {
-      // version style numbering (with earliest file deletion if max files
-      // reached)
-
-      // find existing backup files
-      BackupFilenameFilter bff = new BackupFilenameFilter(basename, suffix,
-              digits,
-              extension);
-      File[] backupFiles = dirFile.listFiles(bff);
-      int nextIndexNum;
-      
-      if (backupFiles.length == 0)
+      else
       {
-        nextIndexNum = 1;
-      } else {
+        // version style numbering (with earliest file deletion if max files
+        // reached)
 
-        // and sort them (based on integer found in the suffix) using a
-        // precomputed Hashmap for speed
-        HashMap bfHashMap = new HashMap<Integer, File>();
-        for (int i = 0; i < backupFiles.length; i++)
-        {
-          File f = backupFiles[i];
-          BackupFilenameParts bfp = new BackupFilenameParts(f, basename, suffix, digits, extension);
-          bfHashMap.put(bfp.indexNum(), f);
-        }
-        TreeMap<Integer, File> bfTreeMap = new TreeMap<>();
-        bfTreeMap.putAll(bfHashMap);
 
         bfTreeMap.values().toArray(backupFiles);
-        
+
         // max value of -1 means keep all backup files
         if (bfTreeMap.size() >= max && max != -1)
         {
           // need to delete some files to keep number of backups to designated
           // max
-          int numToDelete = bfTreeMap.size() - max;
-          File[] filesToDelete = Arrays.copyOfRange(backupFiles, 0,
-                  numToDelete - 1);
-
-          /******************************************
-           * CONFIRM THESE DELETIONS WITH THE USER! *
-           ******************************************/
-          for (int i = 0; i < filesToDelete.length; i++)
-          {
-            File toDelete = filesToDelete[i];
-            toDelete.delete();
-          }
+          int numToDelete = bfTreeMap.size() - max + 1;
+          oldFilesToDelete = Arrays.copyOfRange(backupFiles, 0,
+                  numToDelete);
 
         }
 
         nextIndexNum = bfTreeMap.lastKey() + 1;
 
-        // Let's make the new backup file!! yay, got there at last!
-        String nextBackupFilename = dir + File.separatorChar
-                + BackupFilenameParts.getBackupFilename(nextIndexNum,
-                        basename, suffix, digits, extension);
-        File nextBackupFile = new File(nextBackupFilename);
-        ret = ret && file.renameTo(nextBackupFile);
       }
     }
 
+    if (oldFilesToDelete != null && oldFilesToDelete.length > 0)
+    {
+      // delete old backup/version files
+
+      boolean delete = false;
+      if (confirmDelete)
+      {
+        // Object[] confirmMessageArray = {};
+        StringBuilder confirmMessage = new StringBuilder();
+        confirmMessage.append(MessageManager
+                .getString("label.backupfiles_confirm_delete_old_files"));
+        for (File f : oldFilesToDelete)
+        {
+          confirmMessage.append("\n");
+          confirmMessage.append(f.getName());
+        }
+        if (confirmDeleteExtraInfo != null
+                && confirmDeleteExtraInfo.length() > 0)
+        {
+          confirmMessage.append("\n");
+          confirmMessage.append(confirmDeleteExtraInfo);
+        }
+        int confirm = JvOptionPane.showConfirmDialog(Desktop.desktop,
+                confirmMessage.toString(),
+                MessageManager
+                        .getString("label.backupfiles_confirm_delete"),
+                JvOptionPane.YES_NO_OPTION, JvOptionPane.WARNING_MESSAGE);
+
+        delete = (confirm == JvOptionPane.YES_OPTION);
+      }
+      else
+      {
+        delete = true;
+      }
+
+      if (delete)
+      {
+        for (int i = 0; i < oldFilesToDelete.length; i++)
+        {
+          File fileToDelete = oldFilesToDelete[i];
+          fileToDelete.delete();
+          // System.out.println("DELETING '" + fileToDelete.getName() +
+          // "'");
+        }
+      }
+
+    }
+
+    // Let's make the new backup file!! yay, got there at last!
+    String latestBackupFilename = dir + File.separatorChar
+            + BackupFilenameParts.getBackupFilename(nextIndexNum, basename,
+                    suffix, digits, extension);
+    File latestBackupFile = new File(latestBackupFilename);
+    ret = ret && file.renameTo(latestBackupFile);
+
     return ret;
   }
 
+  public boolean rollBackupsAndRenameTempFile()
+  {
+    boolean write = this.getWriteSuccess();
+    
+    boolean roll = false;
+    if (write) {
+      roll = this.rollBackupFiles();
+    } else {
+      return false;
+    }
+    
+    /*
+     * Not sure that this confirmation is desirable.  By this stage the new file is
+     * already written successfully, but something (e.g. disk full) has happened while 
+     * trying to roll the backup files, and most likely the filename needed will already
+     * be vacant so renaming the temp file is nearly always correct!
+     */
+    if (!roll)
+    {
+      int confirm = JvOptionPane.showConfirmDialog(Desktop.desktop,
+              MessageManager.getString(
+                      "label.backupfiles_confirm_save_file_backupfiles_roll_wrong"),
+              MessageManager.getString("label.backupfiles_confirm_save_file"),
+              JvOptionPane.YES_NO_OPTION, JvOptionPane.WARNING_MESSAGE);
+
+      if (confirm == JvOptionPane.YES_OPTION)
+      {
+        roll = true;
+      }
+    }
+
+    boolean rename = false;
+    if (roll)
+    {
+      rename = this.renameTempFile();
+    }
+
+    return rename;
+  }
+
 }
 
index 90a3cc0..192948d 100755 (executable)
@@ -336,10 +336,13 @@ public class JalviewFileChooser extends JFileChooser
         setSelectedFile(ourselectedFile);
       }
     }
+
     // TODO: ENSURE THAT FILES SAVED WITH A ':' IN THE NAME ARE REFUSED AND THE
-    // USER PROMPTED FOR A NEW FILENAME
+    // USER PROMPTED FOR A NEW FILENAME.
+    // DO NOT need to confirm file overwrite if using backup files (the files
+    // aren't being overwritten!)
     if ((ret == JalviewFileChooser.APPROVE_OPTION)
-            && ourselectedFile.exists())
+            && ourselectedFile.exists() && (!BackupFiles.getEnabled()))
     {
       int confirm = JvOptionPane.showConfirmDialog(parent,
               MessageManager.getString("label.overwrite_existing_file"),
@@ -351,7 +354,6 @@ public class JalviewFileChooser extends JFileChooser
         ret = JalviewFileChooser.CANCEL_OPTION;
       }
 
-      // not happening here now rollBackupFiles(ourselectedFile);
     }
 
     return ret;