Skip to content

Fix selection of table sort order #10250

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 40 commits into from
Sep 2, 2023
Merged
Changes from all commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
593edda
Inline LOGGER.debug
koppor Aug 29, 2023
2ab1c65
Move out work in constructor to method
koppor Aug 29, 2023
1e9a904
Merge remote-tracking branch 'upstream/main' into fix-table-sort-order
koppor Aug 29, 2023
c9af89d
Merge remote-tracking branch 'upstream/main' into fix-table-sort-order
koppor Aug 29, 2023
e999722
Streamline code
koppor Aug 29, 2023
7740b6a
Same comments
koppor Aug 29, 2023
d31f54c
Fix variable name
koppor Aug 29, 2023
c93b0c7
"Flatten" SaveOrder if OrderType.TABLE
koppor Aug 29, 2023
4e679dc
WIP: Show diff in UI
koppor Aug 30, 2023
d5a8a6a
Make it scrollable
koppor Aug 30, 2023
dad6375
WIP: Try to fix content selector diff
koppor Aug 30, 2023
70a7786
Merge remote-tracking branch 'upstream/main' into fix-table-sort-order
koppor Sep 1, 2023
c6620b2
Add some debugging
koppor Sep 1, 2023
5aa6917
Update preferences immediatly after change (and not sometime later)
koppor Sep 1, 2023
81bddcf
Fix serialization of SaveOrder
koppor Sep 1, 2023
fec5865
Add CHANGELOG.md entry
koppor Sep 1, 2023
ac512d9
Fix mapping of SaveOrderConfig
koppor Sep 1, 2023
8190d96
Introduce SelfContainedSaveOrder
koppor Sep 1, 2023
581be07
More SelfContainedSaveOrder
koppor Sep 1, 2023
4b67f1e
Remove ref to PrefsService
calixtus Sep 1, 2023
1a3ea4b
Merge branch 'fix-table-sort-order' of github.com:JabRef/jabref into …
koppor Sep 1, 2023
58e94ae
Compile fix
koppor Sep 1, 2023
244b6c0
Made OrFields NOT extending LinkedHashSet<Field>
koppor Sep 1, 2023
aa9a4fc
Fix hillarious bug
koppor Sep 1, 2023
b2115e9
Fixed tests
calixtus Sep 1, 2023
dfb3ef7
Try to fix FieldComparators for OrFields
koppor Sep 1, 2023
6bede13
Merge branch 'fix-table-sort-order' of github.com:JabRef/jabref into …
koppor Sep 1, 2023
ab4fd00
Fix order of null comparisons
koppor Sep 1, 2023
201137a
Fix checkstyle
koppor Sep 1, 2023
52931e0
Add missing equals, hashcode and toString
koppor Sep 1, 2023
7f626da
Fix checkstyle
koppor Sep 1, 2023
13c6ab5
Merge remote-tracking branch 'upstream/main' into fix-table-sort-order
koppor Sep 1, 2023
e5bc5be
Restore test files
koppor Sep 1, 2023
10da21b
Fix OpenRewrite
koppor Sep 1, 2023
a8cb36f
Fix modernizer
koppor Sep 2, 2023
c026cea
Update CHANGELOG.md
koppor Sep 2, 2023
7eda44f
Merge branch 'main' into fix-table-sort-order
koppor Sep 2, 2023
3433d7c
Merge branch 'main' into fix-table-sort-order
koppor Sep 2, 2023
d478f28
Fix NPE
koppor Sep 2, 2023
8adba2e
Merge branch 'main' into fix-table-sort-order
koppor Sep 2, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -478,5 +478,7 @@ lib/ojdbc.jar
# do not ignore JabRef icons (they are ignored by the macos setting above)
!src/main/java/org/jabref/gui/icon

!**/autosaveandbackup/*.bak

# generated during release process
CHANGELOG.html
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -17,6 +17,8 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve

### Fixed

- It is possible again to use "current table sort order" for the order of entries when saving. [#9869](https://github.com/JabRef/jabref/issues/9869)

### Removed

## [5.10] - 2023-09-02
4 changes: 2 additions & 2 deletions src/main/java/org/jabref/gui/JabRefFrame.java
Original file line number Diff line number Diff line change
@@ -54,6 +54,8 @@
import org.jabref.gui.actions.ActionHelper;
import org.jabref.gui.actions.SimpleCommand;
import org.jabref.gui.actions.StandardActions;
import org.jabref.gui.autosaveandbackup.AutosaveManager;
import org.jabref.gui.autosaveandbackup.BackupManager;
import org.jabref.gui.auximport.NewSubLibraryAction;
import org.jabref.gui.bibtexextractor.ExtractBibtexAction;
import org.jabref.gui.citationkeypattern.GenerateCitationKeyAction;
@@ -118,8 +120,6 @@
import org.jabref.gui.util.BackgroundTask;
import org.jabref.gui.util.DefaultTaskExecutor;
import org.jabref.gui.util.TaskExecutor;
import org.jabref.logic.autosaveandbackup.AutosaveManager;
import org.jabref.logic.autosaveandbackup.BackupManager;
import org.jabref.logic.citationstyle.CitationStyleOutputFormat;
import org.jabref.logic.help.HelpFile;
import org.jabref.logic.importer.IdFetcher;
6 changes: 3 additions & 3 deletions src/main/java/org/jabref/gui/LibraryTab.java
Original file line number Diff line number Diff line change
@@ -26,6 +26,8 @@
import org.jabref.gui.autocompleter.AutoCompletePreferences;
import org.jabref.gui.autocompleter.PersonNameSuggestionProvider;
import org.jabref.gui.autocompleter.SuggestionProviders;
import org.jabref.gui.autosaveandbackup.AutosaveManager;
import org.jabref.gui.autosaveandbackup.BackupManager;
import org.jabref.gui.collab.DatabaseChangeMonitor;
import org.jabref.gui.dialogs.AutosaveUiManager;
import org.jabref.gui.entryeditor.EntryEditor;
@@ -39,8 +41,6 @@
import org.jabref.gui.undo.UndoableRemoveEntries;
import org.jabref.gui.util.BackgroundTask;
import org.jabref.gui.util.DefaultTaskExecutor;
import org.jabref.logic.autosaveandbackup.AutosaveManager;
import org.jabref.logic.autosaveandbackup.BackupManager;
import org.jabref.logic.citationstyle.CitationStyleCache;
import org.jabref.logic.importer.ParserResult;
import org.jabref.logic.importer.util.FileFieldParser;
@@ -290,7 +290,7 @@ public void installAutosaveManagerAndBackupManager() {
autosaveManager.registerListener(new AutosaveUiManager(this));
}
if (isDatabaseReadyForBackup(bibDatabaseContext) && preferencesService.getFilePreferences().shouldCreateBackup()) {
BackupManager.start(bibDatabaseContext, Globals.entryTypesManager, preferencesService);
BackupManager.start(this, bibDatabaseContext, Globals.entryTypesManager, preferencesService);
}
}

Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package org.jabref.logic.autosaveandbackup;
package org.jabref.gui.autosaveandbackup;

import java.util.HashSet;
import java.util.Set;
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package org.jabref.logic.autosaveandbackup;
package org.jabref.gui.autosaveandbackup;

import java.io.IOException;
import java.io.Writer;
@@ -19,6 +19,11 @@
import java.util.concurrent.ScheduledThreadPoolExecutor;
import java.util.concurrent.TimeUnit;

import javafx.scene.control.TableColumn;

import org.jabref.gui.LibraryTab;
import org.jabref.gui.maintable.BibEntryTableViewModel;
import org.jabref.gui.maintable.columns.MainTableColumn;
import org.jabref.logic.bibtex.InvalidFieldValueException;
import org.jabref.logic.exporter.AtomicFileWriter;
import org.jabref.logic.exporter.BibWriter;
@@ -31,6 +36,7 @@
import org.jabref.model.database.event.BibDatabaseContextChangedEvent;
import org.jabref.model.entry.BibEntryTypesManager;
import org.jabref.model.metadata.SaveOrder;
import org.jabref.model.metadata.SelfContainedSaveOrder;
import org.jabref.preferences.PreferencesService;

import com.google.common.eventbus.Subscribe;
@@ -58,17 +64,19 @@ public class BackupManager {
private final ScheduledThreadPoolExecutor executor;
private final CoarseChangeFilter changeFilter;
private final BibEntryTypesManager entryTypesManager;
private final LibraryTab libraryTab;

// Contains a list of all backup paths
// During a write, the less recent backup file is deleted
private final Queue<Path> backupFilesQueue = new LinkedBlockingQueue<>();
private boolean needsBackup = false;

BackupManager(BibDatabaseContext bibDatabaseContext, BibEntryTypesManager entryTypesManager, PreferencesService preferences) {
BackupManager(LibraryTab libraryTab, BibDatabaseContext bibDatabaseContext, BibEntryTypesManager entryTypesManager, PreferencesService preferences) {
this.bibDatabaseContext = bibDatabaseContext;
this.entryTypesManager = entryTypesManager;
this.preferences = preferences;
this.executor = new ScheduledThreadPoolExecutor(2);
this.libraryTab = libraryTab;

changeFilter = new CoarseChangeFilter(bibDatabaseContext);
changeFilter.registerListener(this);
@@ -96,8 +104,8 @@ static Optional<Path> getLatestBackupPath(Path originalPath, Path backupDir) {
*
* @param bibDatabaseContext Associated {@link BibDatabaseContext}
*/
public static BackupManager start(BibDatabaseContext bibDatabaseContext, BibEntryTypesManager entryTypesManager, PreferencesService preferences) {
BackupManager backupManager = new BackupManager(bibDatabaseContext, entryTypesManager, preferences);
public static BackupManager start(LibraryTab libraryTab, BibDatabaseContext bibDatabaseContext, BibEntryTypesManager entryTypesManager, PreferencesService preferences) {
BackupManager backupManager = new BackupManager(libraryTab, bibDatabaseContext, entryTypesManager, preferences);
backupManager.startBackupTask(preferences.getFilePreferences().getBackupDirectory());
runningInstances.add(backupManager);
return backupManager;
@@ -215,18 +223,36 @@ void performBackup(Path backupPath) {

// We opted for "while" to delete backups in case there are more than 10
while (backupFilesQueue.size() >= MAXIMUM_BACKUP_FILE_COUNT) {
Path lessRecentBackupFile = backupFilesQueue.poll();
Path oldestBackupFile = backupFilesQueue.poll();
try {
Files.delete(lessRecentBackupFile);
Files.delete(oldestBackupFile);
} catch (IOException e) {
LOGGER.error("Could not delete backup file {}", lessRecentBackupFile, e);
LOGGER.error("Could not delete backup file {}", oldestBackupFile, e);
}
}

// code similar to org.jabref.gui.exporter.SaveDatabaseAction.saveDatabase
SelfContainedSaveOrder saveOrder = bibDatabaseContext
.getMetaData().getSaveOrder()
.map(so -> {
if (so.getOrderType() == SaveOrder.OrderType.TABLE) {
// We need to "flatten out" SaveOrder.OrderType.TABLE as BibWriter does not have access to preferences
List<TableColumn<BibEntryTableViewModel, ?>> sortOrder = libraryTab.getMainTable().getSortOrder();
return new SelfContainedSaveOrder(
SaveOrder.OrderType.SPECIFIED,
sortOrder.stream()
.filter(col -> col instanceof MainTableColumn<?>)
.map(column -> ((MainTableColumn<?>) column).getModel())
.flatMap(model -> model.getSortCriteria().stream())
.toList());
} else {
return SelfContainedSaveOrder.of(so);
}
})
.orElse(SaveOrder.getDefaultSaveOrder());
SaveConfiguration saveConfiguration = new SaveConfiguration()
.withMakeBackup(false)
.withSaveOrder(bibDatabaseContext.getMetaData().getSaveOrder().orElse(SaveOrder.getDefaultSaveOrder()))
.withSaveOrder(saveOrder)
.withReformatOnSave(preferences.getLibraryPreferences().shouldAlwaysReformatOnSave());

Charset encoding = bibDatabaseContext.getMetaData().getEncoding().orElse(StandardCharsets.UTF_8);
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.jabref.gui.collab.metedatachange;

import javafx.scene.control.Label;
import javafx.scene.control.ScrollPane;
import javafx.scene.layout.VBox;

import org.jabref.gui.collab.DatabaseChangeDetailsView;
@@ -17,20 +18,24 @@ public MetadataChangeDetailsView(MetadataChange metadataChange, PreferencesServi
header.getStyleClass().add("sectionHeader");
container.getChildren().add(header);

for (MetaDataDiff.Difference change : metadataChange.getMetaDataDiff().getDifferences(preferencesService)) {
container.getChildren().add(new Label(getDifferenceString(change)));
for (MetaDataDiff.Difference diff : metadataChange.getMetaDataDiff().getDifferences(preferencesService)) {
container.getChildren().add(new Label(getDifferenceString(diff.differenceType())));
container.getChildren().add(new Label(diff.originalObject().toString()));
container.getChildren().add(new Label(diff.newObject().toString()));
}

setLeftAnchor(container, 8d);
setTopAnchor(container, 8d);
setRightAnchor(container, 8d);
setBottomAnchor(container, 8d);
ScrollPane scrollPane = new ScrollPane(container);
scrollPane.setFitToWidth(true);
getChildren().setAll(scrollPane);

getChildren().setAll(container);
setLeftAnchor(scrollPane, 8d);
setTopAnchor(scrollPane, 8d);
setRightAnchor(scrollPane, 8d);
setBottomAnchor(scrollPane, 8d);
}

private String getDifferenceString(MetaDataDiff.Difference change) {
return switch (change) {
private String getDifferenceString(MetaDataDiff.DifferenceType changeType) {
return switch (changeType) {
case PROTECTED ->
Localization.lang("Library protection");
case GROUPS_ALTERED ->
2 changes: 1 addition & 1 deletion src/main/java/org/jabref/gui/dialogs/BackupUIManager.java
Original file line number Diff line number Diff line change
@@ -8,13 +8,13 @@
import javafx.scene.control.ButtonType;

import org.jabref.gui.DialogService;
import org.jabref.gui.autosaveandbackup.BackupManager;
import org.jabref.gui.backup.BackupResolverDialog;
import org.jabref.gui.collab.DatabaseChange;
import org.jabref.gui.collab.DatabaseChangeList;
import org.jabref.gui.collab.DatabaseChangeResolverFactory;
import org.jabref.gui.collab.DatabaseChangesResolverDialog;
import org.jabref.gui.util.DefaultTaskExecutor;
import org.jabref.logic.autosaveandbackup.BackupManager;
import org.jabref.logic.importer.ImportFormatPreferences;
import org.jabref.logic.importer.OpenDatabase;
import org.jabref.logic.importer.ParserResult;
Original file line number Diff line number Diff line change
@@ -56,7 +56,7 @@ protected Set<Field> determineFieldsToShow(BibEntry entry) {
Set<Field> fields = new LinkedHashSet<>();
if (entryType.isPresent()) {
for (OrFields orFields : entryType.get().getRequiredFields()) {
fields.addAll(orFields);
fields.addAll(orFields.getFields());
}
// Add the edit field for Bibtex-key.
fields.add(InternalField.KEY_FIELD);
41 changes: 32 additions & 9 deletions src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java
Original file line number Diff line number Diff line change
@@ -6,23 +6,27 @@
import java.nio.charset.UnsupportedCharsetException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;

import javafx.scene.control.ButtonBar;
import javafx.scene.control.ButtonType;
import javafx.scene.control.DialogPane;
import javafx.scene.control.TableColumn;
import javafx.scene.layout.VBox;
import javafx.scene.text.Text;

import org.jabref.gui.DialogService;
import org.jabref.gui.JabRefFrame;
import org.jabref.gui.LibraryTab;
import org.jabref.gui.autosaveandbackup.AutosaveManager;
import org.jabref.gui.autosaveandbackup.BackupManager;
import org.jabref.gui.maintable.BibEntryTableViewModel;
import org.jabref.gui.maintable.columns.MainTableColumn;
import org.jabref.gui.util.BackgroundTask;
import org.jabref.gui.util.FileDialogConfiguration;
import org.jabref.logic.autosaveandbackup.AutosaveManager;
import org.jabref.logic.autosaveandbackup.BackupManager;
import org.jabref.logic.exporter.AtomicFileWriter;
import org.jabref.logic.exporter.BibDatabaseWriter;
import org.jabref.logic.exporter.BibWriter;
@@ -38,6 +42,7 @@
import org.jabref.model.database.event.ChangePropagation;
import org.jabref.model.entry.BibEntryTypesManager;
import org.jabref.model.metadata.SaveOrder;
import org.jabref.model.metadata.SelfContainedSaveOrder;
import org.jabref.preferences.PreferencesService;

import org.slf4j.Logger;
@@ -90,11 +95,31 @@ public boolean saveAs(Path file) {
return this.saveAs(file, SaveDatabaseMode.NORMAL);
}

private SelfContainedSaveOrder getSaveOrder() {
return libraryTab.getBibDatabaseContext()
.getMetaData().getSaveOrder()
.map(so -> {
if (so.getOrderType() == SaveOrder.OrderType.TABLE) {
// We need to "flatten out" SaveOrder.OrderType.TABLE as BibWriter does not have access to preferences
List<TableColumn<BibEntryTableViewModel, ?>> sortOrder = libraryTab.getMainTable().getSortOrder();
return new SelfContainedSaveOrder(
SaveOrder.OrderType.SPECIFIED,
sortOrder.stream()
.filter(col -> col instanceof MainTableColumn<?>)
.map(column -> ((MainTableColumn<?>) column).getModel())
.flatMap(model -> model.getSortCriteria().stream())
.toList());
} else {
return SelfContainedSaveOrder.of(so);
}
})
.orElse(SaveOrder.getDefaultSaveOrder());
}

public void saveSelectedAsPlain() {
askForSavePath().ifPresent(path -> {
try {
saveDatabase(path, true, StandardCharsets.UTF_8, BibDatabaseWriter.SaveType.PLAIN_BIBTEX,
libraryTab.getBibDatabaseContext().getMetaData().getSaveOrder().orElse(SaveOrder.getDefaultSaveOrder()));
saveDatabase(path, true, StandardCharsets.UTF_8, BibDatabaseWriter.SaveType.PLAIN_BIBTEX, getSaveOrder());
frame.getFileHistory().newFile(path);
dialogService.notify(Localization.lang("Saved selected to '%0'.", path.toString()));
} catch (SaveException ex) {
@@ -211,9 +236,7 @@ private boolean save(Path targetPath, SaveDatabaseMode mode) {
// Make sure to remember which encoding we used
libraryTab.getBibDatabaseContext().getMetaData().setEncoding(encoding, ChangePropagation.DO_NOT_POST_EVENT);

// Save the database
boolean success = saveDatabase(targetPath, false, encoding, BibDatabaseWriter.SaveType.WITH_JABREF_META_DATA,
libraryTab.getBibDatabaseContext().getMetaData().getSaveOrder().orElse(SaveOrder.getDefaultSaveOrder()));
boolean success = saveDatabase(targetPath, false, encoding, BibDatabaseWriter.SaveType.WITH_JABREF_META_DATA, getSaveOrder());

if (success) {
libraryTab.getUndoManager().markUnchanged();
@@ -231,7 +254,7 @@ private boolean save(Path targetPath, SaveDatabaseMode mode) {
}
}

private boolean saveDatabase(Path file, boolean selectedOnly, Charset encoding, BibDatabaseWriter.SaveType saveType, SaveOrder saveOrder) throws SaveException {
private boolean saveDatabase(Path file, boolean selectedOnly, Charset encoding, BibDatabaseWriter.SaveType saveType, SelfContainedSaveOrder saveOrder) throws SaveException {
// if this code is adapted, please also adapt org.jabref.logic.autosaveandbackup.BackupManager.performBackup

SaveConfiguration saveConfiguration = new SaveConfiguration()
@@ -269,7 +292,7 @@ private boolean saveDatabase(Path file, boolean selectedOnly, Charset encoding,
}
}

private void saveWithDifferentEncoding(Path file, boolean selectedOnly, Charset encoding, Set<Character> encodingProblems, BibDatabaseWriter.SaveType saveType, SaveOrder saveOrder) throws SaveException {
private void saveWithDifferentEncoding(Path file, boolean selectedOnly, Charset encoding, Set<Character> encodingProblems, BibDatabaseWriter.SaveType saveType, SelfContainedSaveOrder saveOrder) throws SaveException {
DialogPane pane = new DialogPane();
VBox vbox = new VBox();
vbox.getChildren().addAll(
Original file line number Diff line number Diff line change
@@ -17,13 +17,13 @@
import org.jabref.gui.LibraryTab;
import org.jabref.gui.StateManager;
import org.jabref.gui.actions.SimpleCommand;
import org.jabref.gui.autosaveandbackup.BackupManager;
import org.jabref.gui.dialogs.BackupUIManager;
import org.jabref.gui.menus.FileHistoryMenu;
import org.jabref.gui.shared.SharedDatabaseUIManager;
import org.jabref.gui.util.BackgroundTask;
import org.jabref.gui.util.DefaultTaskExecutor;
import org.jabref.gui.util.FileDialogConfiguration;
import org.jabref.logic.autosaveandbackup.BackupManager;
import org.jabref.logic.importer.OpenDatabase;
import org.jabref.logic.importer.ParserResult;
import org.jabref.logic.l10n.Localization;
Original file line number Diff line number Diff line change
@@ -5,6 +5,7 @@
import java.util.Comparator;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -67,12 +68,35 @@ public void setValues() {
@Override
public void storeSettings() {
List<Field> metaDataFields = metaData.getContentSelectors().getFieldsWithSelectors();

if (isDefaultMap(fieldKeywordsMap)) {
Iterator<ContentSelector> iterator = metaData.getContentSelectors().getContentSelectors().iterator();
while (iterator.hasNext()) {
metaData.clearContentSelectors(iterator.next().getField());
}
}

fieldKeywordsMap.forEach((field, keywords) -> updateMetaDataContentSelector(metaDataFields, field, keywords));

List<Field> fieldNamesToRemove = filterFieldsToRemove();
fieldNamesToRemove.forEach(metaData::clearContentSelectors);
}

private boolean isDefaultMap(Map<Field, List<String>> 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<Field> getFieldNamesBackingList() {
return fields;
}
Loading