diff --git a/CHANGELOG.md b/CHANGELOG.md index 02603e69334..7d5cfce7070 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv ### Fixed - We fixed an issue where entry type with duplicate fields prevented opening existing libraries with custom entry types [#11127](https://github.com/JabRef/jabref/issues/11127) +- We fixed an issue when the file was flagged as changed on disk in the case of content selectors or groups. [#9064](https://github.com/JabRef/jabref/issues/9064) - We fixed crash on opening the entry editor when auto completion is enabled. [#11188](https://github.com/JabRef/jabref/issues/11188) - We fixed the usage of the key binding for "Clear search" (default: Escape). [#10764](https://github.com/JabRef/jabref/issues/10764). diff --git a/src/main/java/org/jabref/gui/collab/DatabaseChangeDetailsViewFactory.java b/src/main/java/org/jabref/gui/collab/DatabaseChangeDetailsViewFactory.java index fddbdd3b973..a781817fd80 100644 --- a/src/main/java/org/jabref/gui/collab/DatabaseChangeDetailsViewFactory.java +++ b/src/main/java/org/jabref/gui/collab/DatabaseChangeDetailsViewFactory.java @@ -73,7 +73,7 @@ public DatabaseChangeDetailsView create(DatabaseChange databaseChange) { } else if (databaseChange instanceof BibTexStringRename stringRename) { return new BibTexStringRenameDetailsView(stringRename); } else if (databaseChange instanceof MetadataChange metadataChange) { - return new MetadataChangeDetailsView(metadataChange, preferencesService); + return new MetadataChangeDetailsView(metadataChange, preferencesService.getCitationKeyPatternPreferences().getKeyPatterns()); } else if (databaseChange instanceof GroupChange groupChange) { return new GroupChangeDetailsView(groupChange); } else if (databaseChange instanceof PreambleChange preambleChange) { diff --git a/src/main/java/org/jabref/gui/collab/metedatachange/MetadataChangeDetailsView.java b/src/main/java/org/jabref/gui/collab/metedatachange/MetadataChangeDetailsView.java index 3c2a81e3e01..e395337d5a0 100644 --- a/src/main/java/org/jabref/gui/collab/metedatachange/MetadataChangeDetailsView.java +++ b/src/main/java/org/jabref/gui/collab/metedatachange/MetadataChangeDetailsView.java @@ -6,19 +6,19 @@ import org.jabref.gui.collab.DatabaseChangeDetailsView; import org.jabref.logic.bibtex.comparator.MetaDataDiff; +import org.jabref.logic.citationkeypattern.GlobalCitationKeyPatterns; import org.jabref.logic.l10n.Localization; -import org.jabref.preferences.PreferencesService; public final class MetadataChangeDetailsView extends DatabaseChangeDetailsView { - public MetadataChangeDetailsView(MetadataChange metadataChange, PreferencesService preferencesService) { + public MetadataChangeDetailsView(MetadataChange metadataChange, GlobalCitationKeyPatterns globalCitationKeyPatterns) { VBox container = new VBox(15); Label header = new Label(Localization.lang("The following metadata changed:")); header.getStyleClass().add("sectionHeader"); container.getChildren().add(header); - for (MetaDataDiff.Difference diff : metadataChange.getMetaDataDiff().getDifferences(preferencesService)) { + for (MetaDataDiff.Difference diff : metadataChange.getMetaDataDiff().getDifferences(globalCitationKeyPatterns)) { container.getChildren().add(new Label(getDifferenceString(diff.differenceType()))); container.getChildren().add(new Label(diff.originalObject().toString())); container.getChildren().add(new Label(diff.newObject().toString())); @@ -38,7 +38,7 @@ private String getDifferenceString(MetaDataDiff.DifferenceType changeType) { return switch (changeType) { case PROTECTED -> Localization.lang("Library protection"); - case GROUPS_ALTERED -> + case GROUPS -> Localization.lang("Modified groups tree"); case ENCODING -> Localization.lang("Library encoding"); diff --git a/src/main/java/org/jabref/gui/libraryproperties/contentselectors/ContentSelectorViewModel.java b/src/main/java/org/jabref/gui/libraryproperties/contentselectors/ContentSelectorViewModel.java index d0755ec53b1..2f59c1490cb 100644 --- a/src/main/java/org/jabref/gui/libraryproperties/contentselectors/ContentSelectorViewModel.java +++ b/src/main/java/org/jabref/gui/libraryproperties/contentselectors/ContentSelectorViewModel.java @@ -25,20 +25,18 @@ import org.jabref.model.database.BibDatabaseContext; import org.jabref.model.entry.field.Field; import org.jabref.model.entry.field.FieldFactory; -import org.jabref.model.entry.field.StandardField; import org.jabref.model.metadata.ContentSelector; +import org.jabref.model.metadata.ContentSelectors; import org.jabref.model.metadata.MetaData; public class ContentSelectorViewModel implements PropertiesTabViewModel { - private static final List DEFAULT_FIELD_NAMES = List.of(StandardField.AUTHOR, StandardField.JOURNAL, StandardField.KEYWORDS, StandardField.PUBLISHER); - private final MetaData metaData; private final DialogService dialogService; // The map from each field to its predefined strings ("keywords") that can be selected - private final Map> fieldKeywordsMap = new HashMap<>(); + private Map> fieldKeywordsMap = new HashMap<>(); private final ListProperty fields = new SimpleListProperty<>(FXCollections.observableArrayList()); private final ListProperty keywords = new SimpleListProperty<>(FXCollections.observableArrayList()); @@ -53,16 +51,14 @@ public class ContentSelectorViewModel implements PropertiesTabViewModel { @Override public void setValues() { // Populate field names keyword map - metaData.getContentSelectors().getContentSelectors().forEach( - existingContentSelector -> fieldKeywordsMap.put(existingContentSelector.getField(), new ArrayList<>(existingContentSelector.getValues())) - ); + fieldKeywordsMap = ContentSelectors.getFieldKeywordsMap(metaData.getContentSelectors().getContentSelectors()); // Populate Field names list List existingFields = new ArrayList<>(fieldKeywordsMap.keySet()); fields.addAll(existingFields); if (fields.isEmpty()) { - DEFAULT_FIELD_NAMES.forEach(this::addFieldIfUnique); + ContentSelectors.DEFAULT_FIELD_NAMES.forEach(this::addFieldIfUnique); } } @@ -71,7 +67,7 @@ public void storeSettings() { List metaDataFields = metaData.getContentSelectors().getFieldsWithSelectors(); List fieldNamesToRemove; - if (isDefaultMap(fieldKeywordsMap)) { + if (ContentSelectors.isDefaultMap(fieldKeywordsMap)) { // Remove all fields of the content selector fieldNamesToRemove = metaData.getContentSelectorsSorted().stream().map(ContentSelector::getField).toList(); } else { @@ -82,24 +78,6 @@ public void storeSettings() { fieldNamesToRemove.forEach(metaData::clearContentSelectors); } - /** - * Checks whether the given map is the default map, i.e. contains only the default field names and no associated keywords. - */ - private boolean isDefaultMap(Map> fieldKeywordsMap) { - if (fieldKeywordsMap.size() != DEFAULT_FIELD_NAMES.size()) { - return false; - } - for (Field field : DEFAULT_FIELD_NAMES) { - if (!fieldKeywordsMap.containsKey(field)) { - return false; - } - if (!fieldKeywordsMap.get(field).isEmpty()) { - return false; - } - } - return true; - } - public ListProperty getFieldNamesBackingList() { return fields; } @@ -225,7 +203,7 @@ private List determineFieldsToRemove() { // Remove all unset default fields result.addAll(fieldKeywordsMap.entrySet() .stream() - .filter(entry -> DEFAULT_FIELD_NAMES.contains(entry.getKey()) && entry.getValue().isEmpty()).map(Map.Entry::getKey) + .filter(entry -> ContentSelectors.DEFAULT_FIELD_NAMES.contains(entry.getKey()) && entry.getValue().isEmpty()).map(Map.Entry::getKey) .toList()); return result; diff --git a/src/main/java/org/jabref/logic/bibtex/comparator/MetaDataDiff.java b/src/main/java/org/jabref/logic/bibtex/comparator/MetaDataDiff.java index 1a4ae3dabd4..2748be2ec00 100644 --- a/src/main/java/org/jabref/logic/bibtex/comparator/MetaDataDiff.java +++ b/src/main/java/org/jabref/logic/bibtex/comparator/MetaDataDiff.java @@ -2,11 +2,17 @@ import java.util.ArrayList; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.Optional; +import org.jabref.logic.citationkeypattern.CitationKeyPattern; +import org.jabref.logic.citationkeypattern.GlobalCitationKeyPatterns; +import org.jabref.logic.groups.DefaultGroupsFactory; +import org.jabref.model.entry.field.Field; +import org.jabref.model.groups.GroupTreeNode; +import org.jabref.model.metadata.ContentSelectors; import org.jabref.model.metadata.MetaData; -import org.jabref.preferences.PreferencesService; public class MetaDataDiff { public enum DifferenceType { @@ -14,7 +20,7 @@ public enum DifferenceType { DEFAULT_KEY_PATTERN, ENCODING, GENERAL_FILE_DIRECTORY, - GROUPS_ALTERED, + GROUPS, KEY_PATTERNS, LATEX_FILE_DIRECTORY, MODE, @@ -41,28 +47,69 @@ public static Optional compare(MetaData originalMetaData, MetaData if (originalMetaData.equals(newMetaData)) { return Optional.empty(); } else { - return Optional.of(new MetaDataDiff(originalMetaData, newMetaData)); + MetaDataDiff diff = new MetaDataDiff(originalMetaData, newMetaData); + List differences = diff.getDifferences(new GlobalCitationKeyPatterns(CitationKeyPattern.NULL_CITATION_KEY_PATTERN)); + if (differences.isEmpty()) { + return Optional.empty(); + } + return Optional.of(diff); } } + /** + * Checks if given content selectors are empty or default + */ + private static boolean isDefaultContentSelectors(ContentSelectors contentSelectors) { + if (contentSelectors.getContentSelectors().isEmpty()) { + return true; + } + Map> fieldKeywordsMap = ContentSelectors.getFieldKeywordsMap(contentSelectors.getContentSelectors()); + return ContentSelectors.isDefaultMap(fieldKeywordsMap); + } + private void addToListIfDiff(List changes, DifferenceType differenceType, Object originalObject, Object newObject) { if (!Objects.equals(originalObject, newObject)) { + if (differenceType == DifferenceType.CONTENT_SELECTOR) { + ContentSelectors originalContentSelectors = (ContentSelectors) originalObject; + ContentSelectors newContentSelectors = (ContentSelectors) newObject; + if (isDefaultContentSelectors(originalContentSelectors) && isDefaultContentSelectors(newContentSelectors)) { + return; + } + } + if (differenceType == DifferenceType.GROUPS) { + Optional originalGroups = (Optional) originalObject; + Optional newGroups = (Optional) newObject; + if (isDefaultGroup(originalGroups) && isDefaultGroup(newGroups)) { + return; + } + } changes.add(new Difference(differenceType, originalObject, newObject)); } } + private boolean isDefaultGroup(Optional groups) { + if (groups.isEmpty()) { + return true; + } + GroupTreeNode groupRoot = groups.get(); + if (!groupRoot.getChildren().isEmpty()) { + return false; + } + return groupRoot.getGroup().equals(DefaultGroupsFactory.getAllEntriesGroup()); + } + /** * Should be kept in sync with {@link MetaData#equals(Object)} */ - public List getDifferences(PreferencesService preferences) { + public List getDifferences(GlobalCitationKeyPatterns globalCitationKeyPatterns) { List changes = new ArrayList<>(); addToListIfDiff(changes, DifferenceType.PROTECTED, originalMetaData.isProtected(), newMetaData.isProtected()); - addToListIfDiff(changes, DifferenceType.GROUPS_ALTERED, originalMetaData.getGroups(), newMetaData.getGroups()); + addToListIfDiff(changes, DifferenceType.GROUPS, originalMetaData.getGroups(), newMetaData.getGroups()); addToListIfDiff(changes, DifferenceType.ENCODING, originalMetaData.getEncoding(), newMetaData.getEncoding()); addToListIfDiff(changes, DifferenceType.SAVE_SORT_ORDER, originalMetaData.getSaveOrder(), newMetaData.getSaveOrder()); addToListIfDiff(changes, DifferenceType.KEY_PATTERNS, - originalMetaData.getCiteKeyPatterns(preferences.getCitationKeyPatternPreferences().getKeyPatterns()), - newMetaData.getCiteKeyPatterns(preferences.getCitationKeyPatternPreferences().getKeyPatterns())); + originalMetaData.getCiteKeyPatterns(globalCitationKeyPatterns), + newMetaData.getCiteKeyPatterns(globalCitationKeyPatterns)); addToListIfDiff(changes, DifferenceType.USER_FILE_DIRECTORY, originalMetaData.getUserFileDirectories(), newMetaData.getUserFileDirectories()); addToListIfDiff(changes, DifferenceType.LATEX_FILE_DIRECTORY, originalMetaData.getLatexFileDirectories(), newMetaData.getLatexFileDirectories()); addToListIfDiff(changes, DifferenceType.DEFAULT_KEY_PATTERN, originalMetaData.getDefaultCiteKeyPattern(), newMetaData.getDefaultCiteKeyPattern()); @@ -77,6 +124,9 @@ public MetaData getNewMetaData() { return newMetaData; } + /** + * Currently, the groups diff is contained here - and as entry in {@link #getDifferences(GlobalCitationKeyPatterns)} + */ public Optional getGroupDifferences() { return groupDiff; } diff --git a/src/main/java/org/jabref/model/metadata/ContentSelectors.java b/src/main/java/org/jabref/model/metadata/ContentSelectors.java index 4eb2262dbb2..fd28221fc97 100644 --- a/src/main/java/org/jabref/model/metadata/ContentSelectors.java +++ b/src/main/java/org/jabref/model/metadata/ContentSelectors.java @@ -4,16 +4,21 @@ import java.util.Arrays; import java.util.Collections; import java.util.Comparator; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.SortedSet; import java.util.TreeSet; import org.jabref.model.entry.field.Field; import org.jabref.model.entry.field.FieldProperty; +import org.jabref.model.entry.field.StandardField; public class ContentSelectors { + public static final List DEFAULT_FIELD_NAMES = List.of(StandardField.AUTHOR, StandardField.JOURNAL, StandardField.KEYWORDS, StandardField.PUBLISHER); + private final SortedSet contentSelectors; public ContentSelectors() { @@ -113,4 +118,30 @@ public String toString() { ", fieldsWithSelectors=" + getFieldsWithSelectors() + '}'; } + + /** + * Checks whether the given map is the default map, i.e. contains only the default field names and no associated keywords. + */ + public static boolean isDefaultMap(Map> fieldKeywordsMap) { + if (fieldKeywordsMap.size() != ContentSelectors.DEFAULT_FIELD_NAMES.size()) { + return false; + } + for (Field field : ContentSelectors.DEFAULT_FIELD_NAMES) { + if (!fieldKeywordsMap.containsKey(field)) { + return false; + } + if (!fieldKeywordsMap.get(field).isEmpty()) { + return false; + } + } + return true; + } + + public static Map> getFieldKeywordsMap(SortedSet contentSelectors) { + final Map> fieldKeywordsMap = new HashMap<>(); + contentSelectors.forEach( + existingContentSelector -> fieldKeywordsMap.put(existingContentSelector.getField(), new ArrayList<>(existingContentSelector.getValues())) + ); + return fieldKeywordsMap; + } } diff --git a/src/test/java/org/jabref/logic/bibtex/comparator/MetaDataDiffTest.java b/src/test/java/org/jabref/logic/bibtex/comparator/MetaDataDiffTest.java index dcc87088911..cd664323c96 100644 --- a/src/test/java/org/jabref/logic/bibtex/comparator/MetaDataDiffTest.java +++ b/src/test/java/org/jabref/logic/bibtex/comparator/MetaDataDiffTest.java @@ -3,14 +3,18 @@ import java.util.List; import java.util.Optional; +import org.jabref.logic.groups.DefaultGroupsFactory; import org.jabref.model.entry.field.StandardField; +import org.jabref.model.groups.ExplicitGroup; +import org.jabref.model.groups.GroupHierarchyType; +import org.jabref.model.groups.GroupTreeNode; import org.jabref.model.metadata.ContentSelector; import org.jabref.model.metadata.MetaData; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; public class MetaDataDiffTest { @Test @@ -24,10 +28,9 @@ public void compareWithSameContentSelectorsDoesNotReportAnyDiffs() { } @Test - @Disabled public void defaultSettingEqualsEmptySetting() { MetaData one = new MetaData(); - // Field list is from {@link org.jabref.gui.libraryproperties.contentselectors.ContentSelectorViewModel.DEFAULT_FIELD_NAMES} + // Field list is from {@link org.jabref.model.metadata.ContentSelectors.DEFAULT_FIELD_NAMES} one.addContentSelector(new ContentSelector(StandardField.AUTHOR, List.of())); one.addContentSelector(new ContentSelector(StandardField.JOURNAL, List.of())); one.addContentSelector(new ContentSelector(StandardField.PUBLISHER, List.of())); @@ -36,4 +39,25 @@ public void defaultSettingEqualsEmptySetting() { assertEquals(Optional.empty(), MetaDataDiff.compare(one, two)); } + + @Test + public void allEntriesGroupIgnored() { + MetaData one = new MetaData(); + one.setGroups(GroupTreeNode.fromGroup(DefaultGroupsFactory.getAllEntriesGroup())); + MetaData two = new MetaData(); + + assertEquals(Optional.empty(), MetaDataDiff.compare(one, two)); + } + + @Test + public void allEntriesGroupContainingGroupNotIgnored() { + MetaData one = new MetaData(); + GroupTreeNode root = GroupTreeNode.fromGroup(DefaultGroupsFactory.getAllEntriesGroup()); + root.addSubgroup(new ExplicitGroup("ExplicitA", GroupHierarchyType.INCLUDING, ',')); + one.setGroups(root); + + MetaData two = new MetaData(); + + assertNotEquals(Optional.empty(), MetaDataDiff.compare(one, two)); + } }