Skip to content
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

Fix selection of table sort order #10250

Merged
merged 40 commits into from
Sep 2, 2023
Merged
Show file tree
Hide file tree
Changes from 18 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
- We fixed an issue that caused high CPU usage and a zombie process after quitting JabRef because of author names autocompletion [#10159](https://github.com/JabRef/jabref/pull/10159)
- We fixed an issue where files with illegal characters in the filename could be added to JabRef. [#10182](https://github.com/JabRef/jabref/issues/10182)
- We fixed that when editing groups, checked-out properties such as case sensitive and regular expression (under "Free search expression") were not displayed checked. [#10108](https://github.com/JabRef/jabref/issues/10108)
- It is possible again to use "current table sort order" for the order of entries when saving. [#](isDebugEnabled)

### Removed

Expand Down
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;
Expand All @@ -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 ->
Expand Down
26 changes: 20 additions & 6 deletions src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import java.nio.charset.UnsupportedCharsetException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.List;
koppor marked this conversation as resolved.
Show resolved Hide resolved
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -36,8 +37,10 @@
import org.jabref.logic.util.StandardFileType;
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.model.database.event.ChangePropagation;
import org.jabref.model.entry.BibEntry;
koppor marked this conversation as resolved.
Show resolved Hide resolved
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;
Expand Down Expand Up @@ -90,11 +93,24 @@ public boolean saveAs(Path file) {
return this.saveAs(file, SaveDatabaseMode.NORMAL);
}

private SaveOrder getSaveOrder() {
return libraryTab.getBibDatabaseContext()
.getMetaData().getSaveOrder()
.map(so -> {
if (so.getOrderType().equals(SaveOrder.OrderType.TABLE)) {
// We need to "flatten out" SaveOrder.OrderType.TABLE as BibWriter does not have access to preferences
return new SaveOrder(SaveOrder.OrderType.SPECIFIED, preferences.getTableSaveOrder().getSortCriteria());
} else {
return 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) {
Expand Down Expand Up @@ -211,9 +227,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();
Expand All @@ -231,7 +245,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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down
22 changes: 6 additions & 16 deletions src/main/java/org/jabref/gui/maintable/MainTable.java
Original file line number Diff line number Diff line change
Expand Up @@ -144,25 +144,15 @@ public MainTable(MainTableDataModel model,

this.getSortOrder().clear();

/* KEEP for debugging purposes
for (var colModel : mainTablePreferences.getColumnPreferences().getColumnSortOrder()) {
for (var col : this.getColumns()) {
var tablecColModel = ((MainTableColumn<?>) col).getModel();
if (tablecColModel.equals(colModel)) {
LOGGER.debug("Adding sort order for col {} ", col);
this.getSortOrder().add(col);
break;
}
}
}
*/

mainTablePreferences.getColumnPreferences().getColumnSortOrder().forEach(columnModel ->
this.getColumns().stream()
.map(column -> (MainTableColumn<?>) column)
.filter(column -> column.getModel().equals(columnModel))
.findFirst()
.ifPresent(column -> this.getSortOrder().add(column)));
.ifPresent(column -> {
LOGGER.debug("Adding sort order for col {} ", column);
this.getSortOrder().add(column);
}));

if (mainTablePreferences.getResizeColumnsToFit()) {
this.setColumnResizePolicy(new SmartConstrainedResizePolicy());
Expand All @@ -178,7 +168,7 @@ public MainTable(MainTableDataModel model,
this.getStylesheets().add(MainTable.class.getResource("MainTable.css").toExternalForm());

// Store visual state
new PersistenceVisualStateTable(this, mainTablePreferences.getColumnPreferences());
new PersistenceVisualStateTable(this, mainTablePreferences.getColumnPreferences()).addListeners();

setupKeyBindings(keyBindingRepository);

Expand All @@ -198,6 +188,7 @@ public MainTable(MainTableDataModel model,
libraryTab.getUndoManager(),
dialogService,
stateManager);

// Enable the header right-click menu.
new MainTableHeaderContextMenu(this, rightClickMenuFactory).show(true);
}
Expand All @@ -208,7 +199,6 @@ public MainTable(MainTableDataModel model,
* @param sortedColumn The sorted column in {@link MainTable}
* @param keyEvent The pressed character
*/

private void jumpToSearchKey(TableColumn<BibEntryTableViewModel, ?> sortedColumn, KeyEvent keyEvent) {
if ((keyEvent.getCharacter() == null) || (sortedColumn == null)) {
return;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,55 +1,72 @@
package org.jabref.gui.maintable;

import java.util.List;
import java.util.stream.Collectors;

import javafx.beans.InvalidationListener;
import javafx.collections.ListChangeListener;
import javafx.scene.control.TableColumn;
import javafx.scene.control.TableView;

import org.jabref.gui.maintable.columns.MainTableColumn;
import org.slf4j.Logger;
koppor marked this conversation as resolved.
Show resolved Hide resolved
import org.slf4j.LoggerFactory;

/**
* Keep track of changes made to the columns (reordering, resorting, resizing).
*/
public class PersistenceVisualStateTable {

private static final Logger LOGGER = LoggerFactory.getLogger(PersistenceVisualStateTable.class);

protected final TableView<BibEntryTableViewModel> table;
protected final ColumnPreferences preferences;

public PersistenceVisualStateTable(final TableView<BibEntryTableViewModel> table, ColumnPreferences preferences) {
public PersistenceVisualStateTable(TableView<BibEntryTableViewModel> table, ColumnPreferences preferences) {
this.table = table;
this.preferences = preferences;
}

public void addListeners() {
table.getColumns().addListener((InvalidationListener) obs -> updateColumns());
table.getSortOrder().addListener((InvalidationListener) obs -> updateSortOrder());
table.getSortOrder().addListener((ListChangeListener) obs -> updateSortOrder());

// As we store the ColumnModels of the MainTable, we need to add the listener to the ColumnModel properties,
// since the value is bound to the model after the listener to the column itself is called.
table.getColumns().forEach(col ->
((MainTableColumn<?>) col).getModel().widthProperty().addListener(obs -> updateColumns()));
table.getColumns().forEach(col ->
((MainTableColumn<?>) col).getModel().sortTypeProperty().addListener(obs -> updateColumns()));

table.getColumns().stream()
.map(col -> ((MainTableColumn<?>) col).getModel())
.forEach(model -> {
model.widthProperty().addListener(obs -> updateColumns());
model.sortTypeProperty().addListener(obs -> updateColumns());
});
}

/**
* Stores shown columns, their width and their sortType in preferences.
* Stores shown columns, their width and their {@link TableColumn.SortType} in preferences.
* The conversion to the "real" string in the preferences is made at
* {@link org.jabref.preferences.JabRefPreferences#getColumnSortTypesAsStringList(ColumnPreferences)}
*/
private void updateColumns() {
preferences.setColumns(
table.getColumns().stream()
.filter(col -> col instanceof MainTableColumn<?>)
.map(column -> ((MainTableColumn<?>) column).getModel())
.collect(Collectors.toList()));
LOGGER.debug("Updating columns");
preferences.setColumns(toList(table.getColumns()));
}

/**
* Stores the SortOrder of the Table in the preferences. Cannot be combined with updateColumns, because JavaFX
* would provide just an empty list for the sort order on other changes.
* Stores the SortOrder of the Table in the preferences. This includes {@link TableColumn.SortType}.
* <br>
* Cannot be combined with updateColumns, because JavaFX would provide just an empty list for the sort order
* on other changes.
*/
private void updateSortOrder() {
preferences.setColumnSortOrder(
table.getSortOrder().stream()
.filter(col -> col instanceof MainTableColumn<?>)
.map(column -> ((MainTableColumn<?>) column).getModel())
.collect(Collectors.toList()));
LOGGER.debug("Updating sort order");
preferences.setColumnSortOrder(toList(table.getSortOrder()));
}

private List<MainTableColumnModel> toList(List<TableColumn<BibEntryTableViewModel, ?>> columns) {
return columns.stream()
.filter(col -> col instanceof MainTableColumn<?>)
.map(column -> ((MainTableColumn<?>) column).getModel())
.collect(Collectors.toList());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public SearchResultsTable(SearchResultsTableDataModel model,
this.getStylesheets().add(MainTable.class.getResource("MainTable.css").toExternalForm());

// Store visual state
new PersistenceVisualStateTable(this, preferencesService.getSearchDialogColumnPreferences());
new PersistenceVisualStateTable(this, preferencesService.getSearchDialogColumnPreferences()).addListeners();

database.getDatabase().registerListener(this);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -215,18 +215,29 @@ 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
SaveOrder saveOrder = bibDatabaseContext
.getMetaData().getSaveOrder()
.map(so -> {
if (so.getOrderType().equals(SaveOrder.OrderType.TABLE)) {
// We need to "flatten out" SaveOrder.OrderType.TABLE as BibWriter does not have access to preferences
return new SaveOrder(SaveOrder.OrderType.SPECIFIED, preferences.getTableSaveOrder().getSortCriteria());
} else {
return 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);
Expand Down
Loading
Loading