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

Observable Preferences O (Language and FileHistory) #9173

Merged
merged 10 commits into from
Sep 28, 2022
4 changes: 2 additions & 2 deletions src/main/java/org/jabref/gui/JabRefFrame.java
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ public JabRefFrame(Stage mainStage) {
this.undoManager = Globals.undoManager;
this.globalSearchBar = new GlobalSearchBar(this, stateManager, prefs, undoManager, dialogService);
this.pushToApplicationCommand = new PushToApplicationCommand(stateManager, dialogService, prefs);
this.fileHistory = new FileHistoryMenu(prefs, dialogService, getOpenDatabaseAction());
this.fileHistory = new FileHistoryMenu(prefs.getGuiPreferences().getFileHistory(), dialogService, getOpenDatabaseAction());
this.taskExecutor = Globals.TASK_EXECUTOR;
this.importFormatReader = Globals.IMPORT_FORMAT_READER;
this.setOnKeyTyped(key -> {
Expand Down Expand Up @@ -402,7 +402,7 @@ private void tearDownJabRef(List<String> filenames) {
// Here we store the names of all current files. If there is no current file, we remove any
// previously stored filename.
if (filenames.isEmpty()) {
prefs.clearEditedFiles();
prefs.getGuiPreferences().getLastFilesOpened().clear();
} else {
Path focusedDatabase = getCurrentLibraryTab().getBibDatabaseContext()
.getDatabasePath()
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/jabref/gui/JabRefGUI.java
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ private void openDatabases() {
// Remove invalid databases
List<ParserResult> invalidDatabases = bibDatabases.stream()
.filter(ParserResult::isInvalid)
.collect(Collectors.toList());
.toList();
failed.addAll(invalidDatabases);
bibDatabases.removeAll(invalidDatabases);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import javafx.scene.layout.VBox;

import org.jabref.gui.collab.ExternalChangeDetailsView;
import org.jabref.logic.bibtex.comparator.MetaDataDiff;
import org.jabref.logic.l10n.Localization;
import org.jabref.preferences.PreferencesService;

Expand All @@ -16,8 +17,8 @@ public MetadataChangeDetailsView(MetadataChange metadataChange, PreferencesServi
header.getStyleClass().add("sectionHeader");
container.getChildren().add(header);

for (String change : metadataChange.getMetaDataDiff().getDifferences(preferencesService)) {
container.getChildren().add(new Label(change));
for (MetaDataDiff.Difference change : metadataChange.getMetaDataDiff().getDifferences(preferencesService)) {
container.getChildren().add(new Label(getDifferenceString(change)));
}

setLeftAnchor(container, 8d);
Expand All @@ -27,4 +28,33 @@ public MetadataChangeDetailsView(MetadataChange metadataChange, PreferencesServi

getChildren().setAll(container);
}

private String getDifferenceString(MetaDataDiff.Difference change) {
return switch (change) {
case PROTECTED ->
Localization.lang("Library protection");
case GROUPS_ALTERED ->
Localization.lang("Modified groups tree");
case ENCODING ->
Localization.lang("Library encoding");
case SAVE_SORT_ORDER ->
Localization.lang("Save sort order");
case KEY_PATTERNS ->
Localization.lang("Key patterns");
case USER_FILE_DIRECTORY ->
Localization.lang("User-specific file directory");
case LATEX_FILE_DIRECTORY ->
Localization.lang("LaTeX file directory");
case DEFAULT_KEY_PATTERN ->
Localization.lang("Default pattern");
case SAVE_ACTIONS ->
Localization.lang("Save actions");
case MODE ->
Localization.lang("Library mode");
case GENERAL_FILE_DIRECTORY ->
Localization.lang("General file directory");
case CONTENT_SELECTOR ->
Localization.lang("Content selectors");
};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ private StackPane getRelatedArticlesPane(BibEntry entry) {
ProgressIndicator progress = new ProgressIndicator();
progress.setMaxSize(100, 100);

MrDLibFetcher fetcher = new MrDLibFetcher(preferencesService.getLanguage().name(),
MrDLibFetcher fetcher = new MrDLibFetcher(preferencesService.getGeneralPreferences().getLanguage().name(),
Globals.BUILD_INFO.version, preferencesService.getMrDlibPreferences());
BackgroundTask
.wrap(() -> fetcher.performSearch(entry))
Expand Down
13 changes: 5 additions & 8 deletions src/main/java/org/jabref/gui/menus/FileHistoryMenu.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,22 +11,19 @@
import org.jabref.gui.importer.actions.OpenDatabaseAction;
import org.jabref.logic.l10n.Localization;
import org.jabref.logic.util.io.FileHistory;
import org.jabref.preferences.PreferencesService;

public class FileHistoryMenu extends Menu {

private final FileHistory history;
private final PreferencesService preferences;
private final DialogService dialogService;
private final OpenDatabaseAction openDatabaseAction;

public FileHistoryMenu(PreferencesService preferences, DialogService dialogService, OpenDatabaseAction openDatabaseAction) {
public FileHistoryMenu(FileHistory fileHistory, DialogService dialogService, OpenDatabaseAction openDatabaseAction) {
setText(Localization.lang("Recent libraries"));

this.preferences = preferences;
this.history = fileHistory;
this.dialogService = dialogService;
this.openDatabaseAction = openDatabaseAction;
history = preferences.getGuiPreferences().getFileHistory();
if (history.isEmpty()) {
setDisable(true);
} else {
Expand All @@ -46,10 +43,10 @@ public boolean openFileByKey(KeyEvent keyEvent) {
}
char key = keyEvent.getCharacter().charAt(0);
int num = Character.getNumericValue(key);
if (num <= 0 || num > history.getHistory().size()) {
if (num <= 0 || num > history.size()) {
return false;
}
this.openFile(history.getFileAt(Integer.parseInt(keyEvent.getCharacter()) - 1));
this.openFile(history.get(Integer.parseInt(keyEvent.getCharacter()) - 1));
return true;
}

Expand All @@ -66,7 +63,7 @@ public void newFile(Path file) {
private void setItems() {
getItems().clear();
for (int index = 0; index < history.size(); index++) {
addItem(history.getFileAt(index), index + 1);
addItem(history.get(index), index + 1);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public GeneralTabViewModel(DialogService dialogService, PreferencesService prefe

public void setValues() {
languagesListProperty.setValue(new SortedList<>(FXCollections.observableArrayList(Language.values()), Comparator.comparing(Language::getDisplayName)));
selectedLanguageProperty.setValue(preferencesService.getLanguage());
selectedLanguageProperty.setValue(preferencesService.getGeneralPreferences().getLanguage());

encodingsListProperty.setValue(FXCollections.observableArrayList(Encodings.getCharsets()));

Expand All @@ -91,8 +91,8 @@ public void setValues() {

public void storeSettings() {
Language newLanguage = selectedLanguageProperty.getValue();
if (newLanguage != preferencesService.getLanguage()) {
preferencesService.setLanguage(newLanguage);
if (newLanguage != preferencesService.getGeneralPreferences().getLanguage()) {
preferencesService.getGeneralPreferences().setLanguage(newLanguage);
Localization.setLanguage(newLanguage);
restartWarning.add(Localization.lang("Changed language") + ": " + newLanguage.getDisplayName());
}
Expand Down
48 changes: 30 additions & 18 deletions src/main/java/org/jabref/logic/bibtex/comparator/MetaDataDiff.java
Original file line number Diff line number Diff line change
@@ -1,15 +1,27 @@
package org.jabref.logic.bibtex.comparator;

import java.util.ArrayList;
import java.util.List;
import java.util.EnumSet;
import java.util.Objects;
import java.util.Optional;

import org.jabref.logic.l10n.Localization;
import org.jabref.model.metadata.MetaData;
import org.jabref.preferences.PreferencesService;

public class MetaDataDiff {
public enum Difference {
PROTECTED,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would extend the enum here and add the localization as second parameter so you don't need a switch statement and instead you can directly return the message

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would contradict the whole meaning of this commit, to extract the call to the logic package from the model.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But there is something odd

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correcting myself: to reduce the calls to the Localization package, to be able to move the 'mother' class to the model package.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then introduce a map with enum keys and l10n values

GROUPS_ALTERED,
ENCODING,
SAVE_SORT_ORDER,
KEY_PATTERNS,
USER_FILE_DIRECTORY,
LATEX_FILE_DIRECTORY,
DEFAULT_KEY_PATTERN,
SAVE_ACTIONS,
MODE,
GENERAL_FILE_DIRECTORY,
CONTENT_SELECTOR
}

private final Optional<GroupDiff> groupDiff;
private final MetaData originalMetaData;
Expand All @@ -32,44 +44,44 @@ public static Optional<MetaDataDiff> compare(MetaData originalMetaData, MetaData
/**
* @implNote Should be kept in sync with {@link MetaData#equals(Object)}
*/
public List<String> getDifferences(PreferencesService preferences) {
List<String> changes = new ArrayList<>();
public EnumSet<Difference> getDifferences(PreferencesService preferences) {
EnumSet<Difference> changes = EnumSet.noneOf(Difference.class);

if (originalMetaData.isProtected() != newMetaData.isProtected()) {
changes.add(Localization.lang("Library protection"));
changes.add(Difference.PROTECTED);
}
if (!Objects.equals(originalMetaData.getGroups(), newMetaData.getGroups())) {
changes.add(Localization.lang("Modified groups tree"));
changes.add(Difference.GROUPS_ALTERED);
}
if (!Objects.equals(originalMetaData.getEncoding(), newMetaData.getEncoding())) {
changes.add(Localization.lang("Library encoding"));
changes.add(Difference.ENCODING);
}
if (!Objects.equals(originalMetaData.getSaveOrderConfig(), newMetaData.getSaveOrderConfig())) {
changes.add(Localization.lang("Save sort order"));
changes.add(Difference.SAVE_SORT_ORDER);
}
if (!Objects.equals(originalMetaData.getCiteKeyPattern(preferences.getGlobalCitationKeyPattern()), newMetaData.getCiteKeyPattern(preferences.getGlobalCitationKeyPattern()))) {
changes.add(Localization.lang("Key patterns"));
changes.add(Difference.KEY_PATTERNS);
}
if (!Objects.equals(originalMetaData.getUserFileDirectories(), newMetaData.getUserFileDirectories())) {
changes.add(Localization.lang("User-specific file directory"));
changes.add(Difference.USER_FILE_DIRECTORY);
}
if (!Objects.equals(originalMetaData.getLatexFileDirectories(), newMetaData.getLatexFileDirectories())) {
changes.add(Localization.lang("LaTeX file directory"));
changes.add(Difference.LATEX_FILE_DIRECTORY);
}
if (!Objects.equals(originalMetaData.getDefaultCiteKeyPattern(), newMetaData.getDefaultCiteKeyPattern())) {
changes.add(Localization.lang("Default pattern"));
changes.add(Difference.DEFAULT_KEY_PATTERN);
}
if (!Objects.equals(originalMetaData.getSaveActions(), newMetaData.getSaveActions())) {
changes.add(Localization.lang("Save actions"));
changes.add(Difference.SAVE_ACTIONS);
}
if (originalMetaData.getMode() != newMetaData.getMode()) {
changes.add(Localization.lang("Library mode"));
if (!originalMetaData.getMode().equals(newMetaData.getMode())) {
changes.add(Difference.MODE);
}
if (!Objects.equals(originalMetaData.getDefaultFileDirectory(), newMetaData.getDefaultFileDirectory())) {
changes.add(Localization.lang("General file directory"));
changes.add(Difference.GENERAL_FILE_DIRECTORY);
}
if (!Objects.equals(originalMetaData.getContentSelectors(), newMetaData.getContentSelectors())) {
changes.add(Localization.lang("Content selectors"));
changes.add(Difference.CONTENT_SELECTOR);
}
return changes;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ private List<FieldChange> cleanupSingleField(Field fieldKey, BibEntry entry) {
// Run formatter
String newValue = formatter.format(oldValue);

if (oldValue.equals(newValue)) {
if (newValue.equals(oldValue)) {
return Collections.emptyList();
} else {
if (newValue.isEmpty()) {
Expand Down Expand Up @@ -86,7 +86,7 @@ private List<FieldChange> cleanupAllFields(BibEntry entry) {
private List<FieldChange> cleanupAllTextFields(BibEntry entry) {
List<FieldChange> fieldChanges = new ArrayList<>();
Set<Field> fields = new HashSet<>(entry.getFields());
fields.removeAll(FieldFactory.getNotTextFieldNames());
FieldFactory.getNotTextFieldNames().forEach(fields::remove);
for (Field fieldKey : fields) {
if (!fieldKey.equals(InternalField.KEY_FIELD)) {
fieldChanges.addAll(cleanupSingleField(fieldKey, entry));
Expand All @@ -109,11 +109,10 @@ public boolean equals(Object obj) {
if (this == obj) {
return true;
}
if (!(obj instanceof FieldFormatterCleanup)) {
return false;
if (obj instanceof FieldFormatterCleanup that) {
return Objects.equals(field, that.field) && Objects.equals(formatter, that.formatter);
}
FieldFormatterCleanup other = (FieldFormatterCleanup) obj;
return Objects.equals(field, other.field) && Objects.equals(formatter, other.formatter);
return false;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,15 +217,17 @@ public boolean equals(Object obj) {
if (this == obj) {
return true;
}
if (!(obj instanceof FieldFormatterCleanups)) {
return false;
if (obj instanceof FieldFormatterCleanups other) {
return Objects.equals(actions, other.actions) && (enabled == other.enabled);
}
FieldFormatterCleanups other = (FieldFormatterCleanups) obj;
return Objects.equals(actions, other.actions) && (enabled == other.enabled);
return false;
}

@Override
public String toString() {
return "FieldFormatterCleanups [enabled=" + enabled + ", actions=" + actions + "]";
return "FieldFormatterCleanups{" +
"enabled=" + enabled + "," +
"actions=" + actions +
"}";
}
}
45 changes: 28 additions & 17 deletions src/main/java/org/jabref/logic/util/io/FileHistory.java
Original file line number Diff line number Diff line change
@@ -1,50 +1,61 @@
package org.jabref.logic.util.io;

import java.nio.file.Path;
import java.util.ArrayList;
import java.util.List;
import java.util.Objects;

import javafx.collections.FXCollections;
import javafx.collections.ObservableList;
import javafx.collections.ModifiableObservableListBase;

public class FileHistory {
public class FileHistory extends ModifiableObservableListBase<Path> {

private static final int HISTORY_SIZE = 8;

private final ObservableList<Path> history;
private final List<Path> history;

public FileHistory(List<Path> files) {
history = FXCollections.observableList(Objects.requireNonNull(files));
private FileHistory(List<Path> list) {
history = new ArrayList<>(list);
}

@Override
public Path get(int index) {
return history.get(index);
}

public int size() {
return history.size();
}

public boolean isEmpty() {
return history.isEmpty();
@Override
protected void doAdd(int index, Path element) {
history.add(index, element);
}

@Override
protected Path doSet(int index, Path element) {
return history.set(index, element);
}

@Override
protected Path doRemove(int index) {
return history.remove(index);
}

/**
* Adds the file to the top of the list. If it already is in the list, it is merely moved to the top.
*/
public void newFile(Path file) {
removeItem(file);
history.add(0, file);
this.add(0, file);
while (size() > HISTORY_SIZE) {
history.remove(HISTORY_SIZE);
}
}

public Path getFileAt(int index) {
return history.get(index);
}

public void removeItem(Path file) {
history.remove(file);
this.remove(file);
}

public ObservableList<Path> getHistory() {
return history;
public static FileHistory of(List<Path> list) {
return new FileHistory(new ArrayList<>(list));
}
}
Loading