Improve things arround change detection (#5770)

* Dismiss changes hides change message * Show added entries in change dialog * Improve message when current value is null * Remove unused reference file * Apply post-load actions before comparing with currently load database
JabRef · Dec 20, 2019 · 6431099 · 6431099
1 parent 44fdfa7
commit 6431099
Show file tree

Hide file tree

Showing 14 changed files with 64 additions and 90 deletions.
diff --git a/src/main/java/org/jabref/gui/BasePanel.java b/src/main/java/org/jabref/gui/BasePanel.java
@@ -991,10 +991,6 @@ public BibDatabaseContext getBibDatabaseContext() {
         return this.bibDatabaseContext;
     }
 
-    public void markExternalChangesAsResolved() {
-        changeMonitor.ifPresent(DatabaseChangeMonitor::markExternalChangesAsResolved);
-    }
-
     public SidePaneManager getSidePaneManager() {
         return sidePaneManager;
     }
@@ -1059,10 +1055,6 @@ public void resetChangeMonitorAndChangePane() {
         this.getChildren().setAll(changePane);
     }
 
-    public void updateTimeStamp() {
-        changeMonitor.ifPresent(DatabaseChangeMonitor::markAsSaved);
-    }
-
     public void copy() {
         mainTable.copy();
     }

diff --git a/src/main/java/org/jabref/gui/collab/ChangeDisplayDialog.java b/src/main/java/org/jabref/gui/collab/ChangeDisplayDialog.java
@@ -42,13 +42,14 @@ public ChangeDisplayDialog(BibDatabaseContext database, List<DatabaseChangeViewM
         Label rootInfo = new Label(Localization.lang("Select the tree nodes to view and accept or reject changes") + '.');
         infoPanel.setCenter(rootInfo);
 
+        ButtonType dismissChanges = new ButtonType(Localization.lang("Dismiss changes"), ButtonBar.ButtonData.CANCEL_CLOSE);
         getDialogPane().getButtonTypes().setAll(
                 new ButtonType(Localization.lang("Accept changes"), ButtonBar.ButtonData.APPLY),
-                ButtonType.CANCEL
+                dismissChanges
         );
 
         setResultConverter(button -> {
-            if (button == ButtonType.CANCEL) {
+            if (button == dismissChanges) {
                 return false;
             } else {
                 // Perform all accepted changes

diff --git a/src/main/java/org/jabref/gui/collab/ChangeScanner.java b/src/main/java/org/jabref/gui/collab/ChangeScanner.java
@@ -10,8 +10,8 @@
 import org.jabref.logic.bibtex.comparator.BibEntryDiff;
 import org.jabref.logic.bibtex.comparator.BibStringDiff;
 import org.jabref.logic.importer.ImportFormatPreferences;
+import org.jabref.logic.importer.OpenDatabase;
 import org.jabref.logic.importer.ParserResult;
-import org.jabref.logic.importer.fileformat.BibtexImporter;
 import org.jabref.model.database.BibDatabaseContext;
 import org.jabref.model.util.DummyFileUpdateMonitor;
 
@@ -36,9 +36,9 @@ public List<DatabaseChangeViewModel> scanForChanges() {
             List<DatabaseChangeViewModel> changes = new ArrayList<>();
 
             // Parse the modified file
+            // Important: apply all post-load actions
             ImportFormatPreferences importFormatPreferences = Globals.prefs.getImportFormatPreferences();
-            ParserResult result = new BibtexImporter(importFormatPreferences, new DummyFileUpdateMonitor())
-                    .importDatabase(database.getDatabasePath().get(), importFormatPreferences.getEncoding());
+            ParserResult result = OpenDatabase.loadDatabase(database.getDatabasePath().get(), importFormatPreferences, new DummyFileUpdateMonitor());
             BibDatabaseContext databaseOnDisk = result.getDatabaseContext();
 
             // Start looking at changes.

diff --git a/src/main/java/org/jabref/gui/collab/DatabaseChangeMonitor.java b/src/main/java/org/jabref/gui/collab/DatabaseChangeMonitor.java
@@ -1,14 +1,11 @@
 package org.jabref.gui.collab;
 
 import java.io.IOException;
-import java.nio.file.Files;
-import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.List;
 
 import org.jabref.gui.util.BackgroundTask;
 import org.jabref.gui.util.TaskExecutor;
-import org.jabref.logic.util.io.FileUtil;
 import org.jabref.model.database.BibDatabaseContext;
 import org.jabref.model.util.FileUpdateListener;
 import org.jabref.model.util.FileUpdateMonitor;
@@ -22,7 +19,6 @@ public class DatabaseChangeMonitor implements FileUpdateListener {
     private final BibDatabaseContext database;
     private final FileUpdateMonitor fileMonitor;
     private final List<DatabaseChangeListener> listeners;
-    private Path referenceFile;
     private TaskExecutor taskExecutor;
 
     public DatabaseChangeMonitor(BibDatabaseContext database, FileUpdateMonitor fileMonitor, TaskExecutor taskExecutor) {
@@ -34,9 +30,6 @@ public DatabaseChangeMonitor(BibDatabaseContext database, FileUpdateMonitor file
         this.database.getDatabasePath().ifPresent(path -> {
             try {
                 fileMonitor.addListenerForFile(path, this);
-                referenceFile = Files.createTempFile("jabref", ".bib");
-                referenceFile.toFile().deleteOnExit();
-                setAsReference(path);
             } catch (IOException e) {
                 LOGGER.error("Error while trying to monitor " + path, e);
             }
@@ -64,15 +57,4 @@ public void unregister() {
         database.getDatabasePath().ifPresent(file -> fileMonitor.removeListener(file, this));
     }
 
-    public void markExternalChangesAsResolved() {
-        markAsSaved();
-    }
-
-    public void markAsSaved() {
-        database.getDatabasePath().ifPresent(this::setAsReference);
-    }
-
-    private void setAsReference(Path file) {
-        FileUtil.copyFile(file, referenceFile, true);
-    }
 }
diff --git a/src/main/java/org/jabref/gui/collab/DatabaseChangePane.java b/src/main/java/org/jabref/gui/collab/DatabaseChangePane.java
@@ -30,16 +30,13 @@ public DatabaseChangePane(Node parent, BibDatabaseContext database, DatabaseChan
     private void onDatabaseChanged(List<DatabaseChangeViewModel> changes) {
         this.getActions().setAll(
                 new Action(Localization.lang("Dismiss changes"), event -> {
-                    monitor.markExternalChangesAsResolved();
                     this.hide();
                 }),
                 new Action(Localization.lang("Review changes"), event -> {
                     ChangeDisplayDialog changeDialog = new ChangeDisplayDialog(database, changes);
-                    boolean changesHandled = changeDialog.showAndWait().orElse(false);
-                    if (changesHandled) {
-                        monitor.markExternalChangesAsResolved();
-                        this.hide();
-                    }
+                    changeDialog.showAndWait();
+
+                    this.hide();
                 }));
         this.show();
     }

diff --git a/src/main/java/org/jabref/gui/collab/EntryAddChangeViewModel.java b/src/main/java/org/jabref/gui/collab/EntryAddChangeViewModel.java
@@ -13,23 +13,27 @@
 
 class EntryAddChangeViewModel extends DatabaseChangeViewModel {
 
-    private final BibEntry diskEntry;
+    private final BibEntry entry;
 
-    public EntryAddChangeViewModel(BibEntry diskEntry) {
-        super(Localization.lang("Added entry"));
-        this.diskEntry = diskEntry;
+    public EntryAddChangeViewModel(BibEntry entry) {
+        super();
+        this.name = entry.getCiteKeyOptional()
+                         .map(key -> Localization.lang("Added entry") + ": '" + key + '\'')
+                         .orElse(Localization.lang("Added entry"));
+        this.entry = entry;
     }
 
     @Override
     public void makeChange(BibDatabaseContext database, NamedCompound undoEdit) {
-        database.getDatabase().insertEntry(diskEntry);
-        undoEdit.addEdit(new UndoableInsertEntry(database.getDatabase(), diskEntry));
+        database.getDatabase().insertEntry(entry);
+        undoEdit.addEdit(new UndoableInsertEntry(database.getDatabase(), entry));
     }
 
     @Override
     public Node description() {
         PreviewViewer previewViewer = new PreviewViewer(new BibDatabaseContext(), JabRefGUI.getMainFrame().getDialogService(), Globals.stateManager);
-        previewViewer.setEntry(diskEntry);
+        previewViewer.setLayout(Globals.prefs.getPreviewPreferences().getCurrentPreviewStyle());
+        previewViewer.setEntry(entry);
         return previewViewer;
     }
 }
diff --git a/src/main/java/org/jabref/gui/collab/EntryChangeViewModel.java b/src/main/java/org/jabref/gui/collab/EntryChangeViewModel.java
@@ -116,7 +116,11 @@ public Node description() {
                 container.getChildren().add(new Label(Localization.lang("Value cleared externally")));
             }
 
-            container.getChildren().add(new Label(Localization.lang("Current value") + ": " + value));
+            if (StringUtil.isNotBlank(value)) {
+                container.getChildren().add(new Label(Localization.lang("Current value") + ": " + value));
+            } else {
+                container.getChildren().add(new Label(Localization.lang("Current value") + ": " + Localization.lang("empty")));
+            }
 
             return container;
         }

diff --git a/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java b/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java
@@ -135,14 +135,12 @@ private boolean doSave() {
                     SavePreferences.DatabaseSaveType.ALL);
 
             if (success) {
-                panel.updateTimeStamp();
                 panel.getUndoManager().markUnchanged();
                 // (Only) after a successful save the following
                 // statement marks that the base is unchanged
                 // since last save:
                 panel.setNonUndoableChange(false);
                 panel.setBaseChanged(false);
-                panel.markExternalChangesAsResolved();
 
                 // Reset title of tab
                 frame.setTabTitle(panel, panel.getTabTitle(),

diff --git a/src/main/java/org/jabref/gui/importer/actions/AppendDatabaseAction.java b/src/main/java/org/jabref/gui/importer/actions/AppendDatabaseAction.java
@@ -179,7 +179,7 @@ private String openIt(Path file, boolean importEntries, boolean importStrings, b
                         boolean importSelectorWords) throws IOException, KeyCollisionException {
             Globals.prefs.put(JabRefPreferences.WORKING_DIRECTORY, file.getParent().toString());
             // Should this be done _after_ we know it was successfully opened?
-            ParserResult parserResult = OpenDatabase.loadDatabase(file.toFile(),
+        ParserResult parserResult = OpenDatabase.loadDatabase(file,
                     Globals.prefs.getImportFormatPreferences(), Globals.getFileUpdateMonitor());
             AppendDatabaseAction.mergeFromBibtex(panel, parserResult, importEntries, importStrings, importGroups,
                     importSelectorWords);

diff --git a/src/main/java/org/jabref/gui/preview/PreviewViewer.java b/src/main/java/org/jabref/gui/preview/PreviewViewer.java
@@ -173,7 +173,7 @@ public void setEntry(BibEntry newEntry) {
     }
 
     private void update() {
-        if (!entry.isPresent() || layout == null) {
+        if (entry.isEmpty() || layout == null) {
             // Nothing to do
             return;
         }

diff --git a/src/main/java/org/jabref/logic/importer/ImportFormatReader.java b/src/main/java/org/jabref/logic/importer/ImportFormatReader.java
@@ -170,7 +170,7 @@ public UnknownFormatImport importUnknownFormat(Path filePath, FileUpdateMonitor
 
         // First, see if it is a BibTeX file:
         try {
-            ParserResult parserResult = OpenDatabase.loadDatabase(filePath.toFile(), importFormatPreferences, fileMonitor);
+            ParserResult parserResult = OpenDatabase.loadDatabase(filePath, importFormatPreferences, fileMonitor);
             if (parserResult.getDatabase().hasEntries() || !parserResult.getDatabase().hasNoStrings()) {
                 parserResult.setFile(filePath.toFile());
                 return new UnknownFormatImport(ImportFormatReader.BIBTEX_FORMAT, parserResult);

diff --git a/src/main/java/org/jabref/logic/importer/OpenDatabase.java b/src/main/java/org/jabref/logic/importer/OpenDatabase.java
@@ -1,7 +1,9 @@
 package org.jabref.logic.importer;
 
-import java.io.File;
 import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.util.Arrays;
 import java.util.List;
 
@@ -29,31 +31,26 @@ private OpenDatabase() {
      *
      * @param name Name of the BIB-file to open
      * @return ParserResult which never is null
+     * @deprecated use {@link #loadDatabase(Path, ImportFormatPreferences, FileUpdateMonitor)} instead
      */
+    @Deprecated
     public static ParserResult loadDatabase(String name, ImportFormatPreferences importFormatPreferences, FileUpdateMonitor fileMonitor) {
-        File file = new File(name);
-        LOGGER.info("Opening: " + name);
+        LOGGER.debug("Opening: " + name);
+        Path file = Paths.get(name);
 
-        if (!file.exists()) {
+        if (!Files.exists(file)) {
             ParserResult pr = ParserResult.fromErrorMessage(Localization.lang("File not found"));
-            pr.setFile(file);
+            pr.setFile(file.toFile());
 
             LOGGER.error(Localization.lang("Error") + ": " + Localization.lang("File not found"));
             return pr;
         }
 
         try {
-            ParserResult pr = OpenDatabase.loadDatabase(file, importFormatPreferences, fileMonitor);
-            pr.setFile(file);
-            if (pr.hasWarnings()) {
-                for (String aWarn : pr.warnings()) {
-                    LOGGER.warn(aWarn);
-                }
-            }
-            return pr;
+            return OpenDatabase.loadDatabase(file, importFormatPreferences, fileMonitor);
         } catch (IOException ex) {
             ParserResult pr = ParserResult.fromError(ex);
-            pr.setFile(file);
+            pr.setFile(file.toFile());
             LOGGER.error("Problem opening .bib-file", ex);
             return pr;
         }
@@ -62,9 +59,9 @@ public static ParserResult loadDatabase(String name, ImportFormatPreferences imp
     /**
      * Opens a new database.
      */
-    public static ParserResult loadDatabase(File fileToOpen, ImportFormatPreferences importFormatPreferences, FileUpdateMonitor fileMonitor)
+    public static ParserResult loadDatabase(Path fileToOpen, ImportFormatPreferences importFormatPreferences, FileUpdateMonitor fileMonitor)
         throws IOException {
-        ParserResult result = new BibtexImporter(importFormatPreferences, fileMonitor).importDatabase(fileToOpen.toPath(),
+        ParserResult result = new BibtexImporter(importFormatPreferences, fileMonitor).importDatabase(fileToOpen,
                 importFormatPreferences.getEncoding());
 
         if (importFormatPreferences.isKeywordSyncEnabled()) {

diff --git a/src/main/resources/l10n/JabRef_en.properties b/src/main/resources/l10n/JabRef_en.properties
@@ -2094,4 +2094,4 @@ Reset\ All=Reset All
 Column\ type\ %0\ is\ unknown.=Column type %0 is unknown.
 Linked\ identifiers=Linked identifiers
 Special\ field\ type\ %0\ is\ unknown.\ Using\ normal\ column\ type.=Special field type %0 is unknown. Using normal column type.
-
+empty=empty