Skip to content

Commit

Permalink
Migrate KeyBindingRepository WIP
Browse files Browse the repository at this point in the history
  • Loading branch information
calixtus committed May 23, 2024
1 parent 3d58c7f commit 513d577
Show file tree
Hide file tree
Showing 11 changed files with 107 additions and 44 deletions.
5 changes: 1 addition & 4 deletions src/main/java/org/jabref/gui/DefaultInjector.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import java.util.function.Function;

import org.jabref.gui.keyboard.KeyBindingRepository;
import org.jabref.logic.journals.JournalAbbreviationRepository;
import org.jabref.logic.protectedterms.ProtectedTermsLoader;
import org.jabref.model.util.FileUpdateMonitor;
Expand All @@ -22,9 +21,7 @@ public class DefaultInjector implements PresenterFactory {
* Dependencies without default constructor are constructed by hand.
*/
private static Object createDependency(Class<?> clazz) {
if (clazz == KeyBindingRepository.class) {
return Globals.getKeyPrefs();
} else if (clazz == JournalAbbreviationRepository.class) {
if (clazz == JournalAbbreviationRepository.class) {
return Globals.journalAbbreviationRepository;
} else if (clazz == FileUpdateMonitor.class) {
return Globals.getFileUpdateMonitor();
Expand Down
8 changes: 1 addition & 7 deletions src/main/java/org/jabref/gui/Globals.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import org.jabref.logic.util.BuildInfo;
import org.jabref.model.util.DirectoryMonitor;
import org.jabref.model.util.FileUpdateMonitor;
import org.jabref.preferences.PreferencesService;

import com.airhacks.afterburner.injection.Injector;

Expand Down Expand Up @@ -42,7 +41,6 @@ public class Globals {
public static ProtectedTermsLoader protectedTermsLoader;

private static ClipBoardManager clipBoardManager = null;
private static KeyBindingRepository keyBindingRepository;

private static DefaultFileUpdateMonitor fileUpdateMonitor;
private static DefaultDirectoryMonitor directoryMonitor;
Expand All @@ -52,11 +50,7 @@ private Globals() {

// Key binding preferences
public static synchronized KeyBindingRepository getKeyPrefs() {
if (keyBindingRepository == null) {
PreferencesService preferences = Injector.instantiateModelOrService(PreferencesService.class);
keyBindingRepository = preferences.getKeyBindingRepository();
}
return keyBindingRepository;
return Injector.instantiateModelOrService(KeyBindingRepository.class);
}

public static synchronized ClipBoardManager getClipboardManager() {
Expand Down
12 changes: 7 additions & 5 deletions src/main/java/org/jabref/gui/JabRefGUI.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import org.jabref.gui.frame.JabRefFrame;
import org.jabref.gui.help.VersionWorker;
import org.jabref.gui.icon.IconTheme;
import org.jabref.gui.keyboard.KeyBindingRepository;
import org.jabref.gui.keyboard.TextInputKeyBindings;
import org.jabref.gui.openoffice.OOBibBaseConnect;
import org.jabref.gui.remote.CLIMessageHandler;
Expand Down Expand Up @@ -81,6 +82,8 @@ public void start(Stage stage) {

JabRefGUI.stateManager = new StateManager();

Injector.setModelOrService(KeyBindingRepository.class, preferencesService.getKeyBindingRepository());

JabRefGUI.themeManager = new ThemeManager(
preferencesService.getWorkspacePreferences(),
fileUpdateMonitor,
Expand Down Expand Up @@ -266,13 +269,12 @@ private boolean isWindowPositionOutOfBounds() {
preferencesService.getGuiPreferences().getPositionY());
}


// Background tasks
public void startBackgroundTasks() {
// TODO Currently deactivated due to incompatibilities in XML
/* if (Globals.prefs.getTelemetryPreferences().shouldCollectTelemetry() && !GraphicsEnvironment.isHeadless()) {
Telemetry.start(prefs.getTelemetryPreferences());
} */
// TODO: Currently deactivated due to incompatibilities in XML
// if (Globals.prefs.getTelemetryPreferences().shouldCollectTelemetry() && !GraphicsEnvironment.isHeadless()) {
// Telemetry.start(prefs.getTelemetryPreferences());
// }
RemotePreferences remotePreferences = preferencesService.getRemotePreferences();
if (remotePreferences.useRemoteServer()) {
Globals.REMOTE_LISTENER.openAndStart(
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/jabref/gui/LibraryTab.java
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,7 @@ private void createMainTable() {
preferencesService,
dialogService,
stateManager,
Globals.getKeyPrefs(),
preferencesService.getKeyBindingRepository(),
Globals.getClipboardManager(),
entryTypesManager,
taskExecutor,
Expand Down
2 changes: 0 additions & 2 deletions src/main/java/org/jabref/gui/entryeditor/SourceTab.java
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ public class SourceTab extends EntryEditorTab {
private final ImportFormatPreferences importFormatPreferences;
private final FileUpdateMonitor fileMonitor;
private final DialogService dialogService;
private final StateManager stateManager;
private final BibEntryTypesManager entryTypesManager;
private final KeyBindingRepository keyBindingRepository;
private Optional<Pattern> searchHighlightPattern = Optional.empty();
Expand Down Expand Up @@ -117,7 +116,6 @@ public SourceTab(BibDatabaseContext bibDatabaseContext,
this.importFormatPreferences = importFormatPreferences;
this.fileMonitor = fileMonitor;
this.dialogService = dialogService;
this.stateManager = stateManager;
this.entryTypesManager = entryTypesManager;
this.keyBindingRepository = keyBindingRepository;

Expand Down
22 changes: 13 additions & 9 deletions src/main/java/org/jabref/gui/keyboard/KeyBindingRepository.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,30 +11,32 @@
import java.util.TreeMap;
import java.util.stream.Collectors;

import javafx.beans.property.MapProperty;
import javafx.beans.property.SimpleMapProperty;
import javafx.collections.FXCollections;
import javafx.scene.input.KeyCode;
import javafx.scene.input.KeyCombination;
import javafx.scene.input.KeyEvent;

import org.jabref.gui.Globals;
import org.jabref.logic.util.OS;

public class KeyBindingRepository {

/**
* sorted by localization
*/
private final SortedMap<KeyBinding, String> bindings;
private final MapProperty<KeyBinding, String> bindings;

public KeyBindingRepository() {
this(Collections.emptyList(), Collections.emptyList());
}

public KeyBindingRepository(SortedMap<KeyBinding, String> bindings) {
this.bindings = bindings;
this.bindings = new SimpleMapProperty<>(FXCollections.observableMap(bindings));
}

public KeyBindingRepository(List<String> bindNames, List<String> bindings) {
this.bindings = new TreeMap<>(Comparator.comparing(KeyBinding::getLocalization));
this.bindings = new SimpleMapProperty<>(FXCollections.observableMap(new TreeMap<>(Comparator.comparing(KeyBinding::getLocalization))));

if ((bindNames.isEmpty()) || (bindings.isEmpty()) || (bindNames.size() != bindings.size())) {
// Use default key bindings
Expand Down Expand Up @@ -141,11 +143,9 @@ private Set<KeyBinding> mapToKeyBindings(KeyEvent keyEvent) {
* Used if a keyboard shortcut leads to multiple actions (e.g., ESC for closing a dialog and clearing the search).
*/
public boolean matches(KeyEvent event, KeyBinding keyBinding) {
return Globals.getKeyPrefs().mapToKeyBindings(event)
.stream()
.filter(binding -> binding == keyBinding)
.findFirst()
.isPresent();
return mapToKeyBindings(event)
.stream()
.anyMatch(binding -> binding == keyBinding);
}

public Optional<KeyCombination> getKeyCombination(KeyBinding bindName) {
Expand Down Expand Up @@ -179,6 +179,10 @@ public List<String> getBindings() {
return new ArrayList<>(bindings.values());
}

public MapProperty<KeyBinding, String> getBindingsProperty() {
return bindings;
}

@Override
public boolean equals(Object o) {
if (this == o) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
public class KeyBindingsTabViewModel implements PreferenceTabViewModel {

private final KeyBindingRepository keyBindingRepository;
private final KeyBindingRepository initialKeyBindingRepository;
private final PreferencesService preferences;
private final OptionalObjectProperty<KeyBindingViewModel> selectedKeyBinding = OptionalObjectProperty.empty();
private final ObjectProperty<KeyBindingViewModel> rootKeyBinding = new SimpleObjectProperty<>();
Expand All @@ -40,8 +39,7 @@ public class KeyBindingsTabViewModel implements PreferenceTabViewModel {
private final List<String> restartWarning = new ArrayList<>();

public KeyBindingsTabViewModel(KeyBindingRepository keyBindingRepository, DialogService dialogService, PreferencesService preferences) {
this.keyBindingRepository = Objects.requireNonNull(keyBindingRepository);
this.initialKeyBindingRepository = new KeyBindingRepository(keyBindingRepository.getKeyBindings());
this.keyBindingRepository = new KeyBindingRepository(keyBindingRepository.getKeyBindings());
this.dialogService = Objects.requireNonNull(dialogService);
this.preferences = Objects.requireNonNull(preferences);

Expand Down Expand Up @@ -85,9 +83,8 @@ public void setNewBindingForCurrent(KeyEvent event) {
}

public void storeSettings() {
preferences.storeKeyBindingRepository(keyBindingRepository);

if (!keyBindingRepository.equals(initialKeyBindingRepository)) {
if (!keyBindingRepository.equals(preferences.getKeyBindingRepository())) {
preferences.getKeyBindingRepository().getBindingsProperty().set(keyBindingRepository.getBindingsProperty());
restartWarning.add(Localization.lang("Keyboard shortcuts changed"));
}
}
Expand Down
26 changes: 18 additions & 8 deletions src/main/java/org/jabref/preferences/JabRefPreferences.java
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@
import com.airhacks.afterburner.injection.Injector;
import com.github.javakeyring.Keyring;
import com.github.javakeyring.PasswordAccessException;
import com.google.common.annotations.VisibleForTesting;
import com.tobiasdiez.easybind.EasyBind;
import jakarta.inject.Singleton;
import org.jvnet.hk2.annotations.Service;
Expand Down Expand Up @@ -511,6 +512,8 @@ public class JabRefPreferences implements PreferencesService {
private FieldPreferences fieldPreferences;
private MergeDialogPreferences mergeDialogPreferences;

private KeyBindingRepository keyBindingRepository;

// The constructor is made private to enforce this as a singleton class:
private JabRefPreferences() {
try {
Expand Down Expand Up @@ -867,11 +870,13 @@ public static JabRefPreferences getInstance() {
// Common serializer logic
//*************************************************************************************************************

private static String convertListToString(List<String> value) {
@VisibleForTesting
static String convertListToString(List<String> value) {
return value.stream().map(val -> StringUtil.quote(val, STRINGLIST_DELIMITER.toString(), '\\')).collect(Collectors.joining(STRINGLIST_DELIMITER.toString()));
}

private static List<String> convertStringToList(String toConvert) {
@VisibleForTesting
static List<String> convertStringToList(String toConvert) {
if (StringUtil.isBlank(toConvert)) {
return Collections.emptyList();
}
Expand Down Expand Up @@ -1220,13 +1225,18 @@ public LayoutFormatterPreferences getLayoutFormatterPreferences() {

@Override
public KeyBindingRepository getKeyBindingRepository() {
return new KeyBindingRepository(getStringList(BIND_NAMES), getStringList(BINDINGS));
}
if (keyBindingRepository != null) {
return keyBindingRepository;
}

@Override
public void storeKeyBindingRepository(KeyBindingRepository keyBindingRepository) {
putStringList(BIND_NAMES, keyBindingRepository.getBindNames());
putStringList(BINDINGS, keyBindingRepository.getBindings());
keyBindingRepository = new KeyBindingRepository(getStringList(BIND_NAMES), getStringList(BINDINGS));

EasyBind.listen(keyBindingRepository.getBindingsProperty(), (obs, oldValue, newValue) -> {
putStringList(BIND_NAMES, keyBindingRepository.getBindNames());
putStringList(BINDINGS, keyBindingRepository.getBindings());
});

return keyBindingRepository;
}

@Override
Expand Down
2 changes: 0 additions & 2 deletions src/main/java/org/jabref/preferences/PreferencesService.java
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,6 @@ public interface PreferencesService {

JournalAbbreviationPreferences getJournalAbbreviationPreferences();

void storeKeyBindingRepository(KeyBindingRepository keyBindingRepository);

KeyBindingRepository getKeyBindingRepository();

FilePreferences getFilePreferences();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package org.jabref.gui.keyboard;

import java.util.List;
import java.util.stream.Stream;

import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;

import static org.junit.jupiter.api.Assertions.assertEquals;

class KeyBindingRepositoryTest {
private static Stream<Arguments> provideTestData() {
return Stream.of(
// Correctly mapped
Arguments.of(
List.of(KeyBinding.ABBREVIATE, KeyBinding.NEW_TECHREPORT, KeyBinding.PASTE),
List.of("ctrl+1", "alt+2", "shift+3")
),

// Defaults on faulty data
Arguments.of(
List.of(KeyBinding.ABBREVIATE, KeyBinding.NEW_TECHREPORT, KeyBinding.PASTE),
List.of(KeyBinding.ABBREVIATE.getDefaultKeyBinding(), KeyBinding.NEW_TECHREPORT.getDefaultKeyBinding())
));
}

@ParameterizedTest
@MethodSource("provideTestData")
void parseStringLists(List<KeyBinding> keybindings, List<String> bindings) {
List<String> bindNames = keybindings.stream().map(KeyBinding::getConstant).toList();
KeyBindingRepository keyBindingRepository = new KeyBindingRepository(bindNames, bindings);

assertEquals(keyBindingRepository.get(bindNames.get(0)), bindings.get(0));
assertEquals(keyBindingRepository.get(bindNames.get(1)), bindings.get(1));
}
}
26 changes: 26 additions & 0 deletions src/test/java/org/jabref/preferences/JabRefPreferencesTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package org.jabref.preferences;

import java.util.List;
import java.util.stream.Stream;

import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;

import static org.junit.jupiter.api.Assertions.assertEquals;

class JabRefPreferencesTest {
private static Stream<Arguments> provideTestData() {
return Stream.of(
Arguments.of(List.of("A", "B", "C", "D"), "A;B;C;D"),
Arguments.of(List.of("A", "B", "C", ""), "A;B;C;")
);
}

@ParameterizedTest
@MethodSource("provideTestData")
void roundTrip(List<String> sampleList, String sampleString) {
assertEquals(sampleString, JabRefPreferences.convertListToString(sampleList));
assertEquals(sampleList, JabRefPreferences.convertStringToList(sampleString));
}
}

0 comments on commit 513d577

Please sign in to comment.