From d0c5f2ebbd1c7664332b6a95a5bb93767f8bc1b2 Mon Sep 17 00:00:00 2001 From: Siedlerchr Date: Sun, 19 May 2019 22:18:59 +0200 Subject: [PATCH] Get rid of SearchQueryHighlightObservable Use SearchQuery directly TODO: fix updating of search results --- .../java/org/jabref/gui/StateManager.java | 23 ----- .../jabref/gui/entryeditor/EntryEditor.java | 8 -- .../org/jabref/gui/entryeditor/SourceTab.java | 32 ++++--- .../org/jabref/gui/preview/PreviewViewer.java | 38 ++++++--- .../jabref/gui/search/GlobalSearchBar.java | 20 ++--- .../org/jabref/logic/search/SearchQuery.java | 25 ++++++ .../search/SearchQueryHighlightListener.java | 20 ----- .../SearchQueryHighlightObservable.java | 85 ------------------- .../SearchQueryHighlightObservableTest.java | 59 ------------- .../jabref/logic/search/SearchQueryTest.java | 13 ++- 10 files changed, 95 insertions(+), 228 deletions(-) delete mode 100644 src/main/java/org/jabref/logic/search/SearchQueryHighlightListener.java delete mode 100644 src/main/java/org/jabref/logic/search/SearchQueryHighlightObservable.java delete mode 100644 src/test/java/org/jabref/logic/search/SearchQueryHighlightObservableTest.java diff --git a/src/main/java/org/jabref/gui/StateManager.java b/src/main/java/org/jabref/gui/StateManager.java index a73886d43a8..37e4bb68e52 100644 --- a/src/main/java/org/jabref/gui/StateManager.java +++ b/src/main/java/org/jabref/gui/StateManager.java @@ -16,8 +16,6 @@ import org.jabref.gui.util.OptionalObjectProperty; import org.jabref.logic.search.SearchQuery; -import org.jabref.logic.search.SearchQueryHighlightListener; -import org.jabref.logic.search.SearchQueryHighlightObservable; import org.jabref.model.database.BibDatabaseContext; import org.jabref.model.entry.BibEntry; import org.jabref.model.groups.GroupTreeNode; @@ -39,7 +37,6 @@ public class StateManager { private final ObservableMap> selectedGroups = FXCollections.observableHashMap(); private final OptionalObjectProperty activeSearchQuery = OptionalObjectProperty.empty(); private final IntegerProperty searchResultSize = new SimpleIntegerProperty(); - private final SearchQueryHighlightObservable searchQueryHighlightObservable = new SearchQueryHighlightObservable(); public StateManager() { activeGroups.bind(Bindings.valueAt(selectedGroups, activeDatabase.orElse(null))); @@ -99,24 +96,4 @@ public void setSearchQuery(SearchQuery searchQuery) { public IntegerProperty searchResultSizeProperty() { return searchResultSize; } - - public SearchQueryHighlightObservable getSearchQueryHighlightObservable() { - return searchQueryHighlightObservable; - } - - public void fireSearchQueryHighlightEvent() { - if (activeSearchQuery.get().isPresent()) { - searchQueryHighlightObservable.fireSearchlistenerEvent(activeSearchQuery.getValue().get()); - } - } - - public void resetSearchQueryHighlightObservable() - { - searchQueryHighlightObservable.reset(); - } - - public void addSearchQueryHighlightListener(SearchQueryHighlightListener listener) { - searchQueryHighlightObservable.removeSearchListener(listener); - searchQueryHighlightObservable.addSearchListener(listener); - } } diff --git a/src/main/java/org/jabref/gui/entryeditor/EntryEditor.java b/src/main/java/org/jabref/gui/entryeditor/EntryEditor.java index eb65cdc0cff..962d26832c4 100644 --- a/src/main/java/org/jabref/gui/entryeditor/EntryEditor.java +++ b/src/main/java/org/jabref/gui/entryeditor/EntryEditor.java @@ -46,7 +46,6 @@ import org.jabref.logic.help.HelpFile; import org.jabref.logic.importer.EntryBasedFetcher; import org.jabref.logic.importer.WebFetchers; -import org.jabref.logic.search.SearchQueryHighlightListener; import org.jabref.model.database.BibDatabaseContext; import org.jabref.model.entry.BibEntry; import org.jabref.model.util.FileUpdateMonitor; @@ -74,7 +73,6 @@ public class EntryEditor extends BorderPane { private final BibDatabaseContext databaseContext; private final CountingUndoManager undoManager; private final BasePanel panel; - private final List searchListeners = new ArrayList<>(); private Subscription typeSubscription; private final List tabs; private final FileUpdateMonitor fileMonitor; @@ -403,12 +401,6 @@ private void fetchAndMerge(EntryBasedFetcher fetcher) { new FetchAndMergeEntry(panel, taskExecutor).fetchAndMerge(entry, fetcher); } - void addSearchListener(SearchQueryHighlightListener listener) { - // TODO: Highlight search text in entry editors - searchListeners.add(listener); - - } - public void setFocusToField(String fieldName) { DefaultTaskExecutor.runInJavaFXThread(() -> { for (Tab tab : tabbed.getTabs()) { diff --git a/src/main/java/org/jabref/gui/entryeditor/SourceTab.java b/src/main/java/org/jabref/gui/entryeditor/SourceTab.java index 7c04527c3e1..8c967f132de 100644 --- a/src/main/java/org/jabref/gui/entryeditor/SourceTab.java +++ b/src/main/java/org/jabref/gui/entryeditor/SourceTab.java @@ -5,12 +5,16 @@ import java.io.StringWriter; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.regex.Matcher; +import java.util.regex.Pattern; import javax.swing.undo.UndoManager; import javafx.beans.property.ObjectProperty; import javafx.beans.property.SimpleObjectProperty; +import javafx.beans.value.ChangeListener; +import javafx.beans.value.ObservableValue; import javafx.collections.ListChangeListener; import javafx.geometry.Point2D; import javafx.scene.control.Tooltip; @@ -33,6 +37,7 @@ import org.jabref.logic.importer.ParserResult; import org.jabref.logic.importer.fileformat.BibtexParser; import org.jabref.logic.l10n.Localization; +import org.jabref.logic.search.SearchQuery; import org.jabref.model.database.BibDatabase; import org.jabref.model.database.BibDatabaseContext; import org.jabref.model.database.BibDatabaseMode; @@ -61,6 +66,7 @@ public class SourceTab extends EntryEditorTab { private final DialogService dialogService; private final StateManager stateManager; + private Optional searchHighlightPattern = Optional.empty(); private CodeArea codeArea; public SourceTab(BibDatabaseContext bibDatabaseContext, CountingUndoManager undoManager, LatexFieldFormatterPreferences fieldFormatterPreferences, ImportFormatPreferences importFormatPreferences, FileUpdateMonitor fileMonitor, DialogService dialogService, StateManager stateManager) { @@ -75,17 +81,23 @@ public SourceTab(BibDatabaseContext bibDatabaseContext, CountingUndoManager undo this.dialogService = dialogService; this.stateManager = stateManager; - stateManager.addSearchQueryHighlightListener(highlightPattern -> { - if (highlightPattern.isPresent() && codeArea != null) { - codeArea.setStyleClass(0, codeArea.getLength(), "text"); - Matcher matcher = highlightPattern.get().matcher(codeArea.getText()); - while (matcher.find()) { - for (int i = 0; i <= matcher.groupCount(); i++) { - codeArea.setStyleClass(matcher.start(), matcher.end(), "search"); - } + stateManager.activeSearchQueryProperty().addListener((observable, oldValue, newValue) -> { + searchHighlightPattern = newValue.flatMap(SearchQuery::getPatternForWords); + highlightSearchPattern(); + }); + + } + + private void highlightSearchPattern() { + if (searchHighlightPattern.isPresent() && codeArea != null) { + codeArea.setStyleClass(0, codeArea.getLength(), "text"); + Matcher matcher = searchHighlightPattern.get().matcher(codeArea.getText()); + while (matcher.find()) { + for (int i = 0; i <= matcher.groupCount(); i++) { + codeArea.setStyleClass(matcher.start(), matcher.end(), "search"); } } - }); + } } private static String getSourceString(BibEntry entry, BibDatabaseMode type, LatexFieldFormatterPreferences fieldFormatterPreferences) throws IOException { @@ -170,7 +182,7 @@ protected void bindToEntry(BibEntry entry) { codeArea.clear(); try { codeArea.appendText(getSourceString(entry, mode, fieldFormatterPreferences)); - stateManager.fireSearchQueryHighlightEvent(); + highlightSearchPattern(); } catch (IOException ex) { codeArea.setEditable(false); diff --git a/src/main/java/org/jabref/gui/preview/PreviewViewer.java b/src/main/java/org/jabref/gui/preview/PreviewViewer.java index bbc14907cec..bdac1d40cab 100644 --- a/src/main/java/org/jabref/gui/preview/PreviewViewer.java +++ b/src/main/java/org/jabref/gui/preview/PreviewViewer.java @@ -2,10 +2,11 @@ import java.util.Objects; import java.util.Optional; +import java.util.regex.Pattern; import javafx.beans.InvalidationListener; import javafx.beans.Observable; -import javafx.beans.value.ObservableValue; +import javafx.beans.value.ChangeListener; import javafx.concurrent.Worker; import javafx.print.PrinterJob; import javafx.scene.control.ScrollPane; @@ -21,6 +22,7 @@ import org.jabref.logic.citationstyle.PreviewLayout; import org.jabref.logic.exporter.ExporterFactory; import org.jabref.logic.l10n.Localization; +import org.jabref.logic.search.SearchQuery; import org.jabref.model.database.BibDatabaseContext; import org.jabref.model.entry.BibEntry; @@ -57,6 +59,9 @@ public class PreviewViewer extends ScrollPane implements InvalidationListener { */ private Optional entry = Optional.empty(); private BibDatabaseContext database; + private boolean registered; + + private Optional searchHighlightPattern = Optional.empty(); /** * @param database Used for resolving strings and pdf directories for links. @@ -72,22 +77,31 @@ public PreviewViewer(BibDatabaseContext database, DialogService dialogService, S setContent(previewView); previewView.setContextMenuEnabled(false); - previewView.getEngine().getLoadWorker().stateProperty().addListener((ObservableValue observable, - Worker.State oldValue, - Worker.State newValue) -> { + previewView.getEngine().getLoadWorker().stateProperty().addListener((observable, oldValue, newValue) -> { + if (newValue != Worker.State.SUCCEEDED) { return; } - stateManager.addSearchQueryHighlightListener(highlightPattern -> { - if (highlightPattern.isPresent()) { - String pattern = highlightPattern.get().pattern().replace("\\Q", "").replace("\\E", ""); + if (!registered) { + stateManager.activeSearchQueryProperty().addListener(listener); + registered = true; + } + highlightSearchPattern(); + }); + + } - previewView.getEngine().executeScript("highlight('" + pattern + "');"); + private ChangeListener> listener = (queryObservable, queryOldValue, queryNewValue) -> { + searchHighlightPattern = queryNewValue.flatMap(SearchQuery::getPatternForWords); + highlightSearchPattern(); + }; - } - }); - }); + private void highlightSearchPattern() { + if (searchHighlightPattern.isPresent()) { + String pattern = searchHighlightPattern.get().pattern().replace("\\Q", "").replace("\\E", ""); + previewView.getEngine().executeScript("highlight('" + pattern + "');"); + } } public void setLayout(PreviewLayout newLayout) { @@ -109,8 +123,8 @@ public void setEntry(BibEntry newEntry) { for (Observable observable : newEntry.getObservables()) { observable.addListener(this); } - update(); + } private void update() { diff --git a/src/main/java/org/jabref/gui/search/GlobalSearchBar.java b/src/main/java/org/jabref/gui/search/GlobalSearchBar.java index 33a99a55961..8ab2a67e4c2 100644 --- a/src/main/java/org/jabref/gui/search/GlobalSearchBar.java +++ b/src/main/java/org/jabref/gui/search/GlobalSearchBar.java @@ -46,7 +46,6 @@ import org.jabref.gui.util.DefaultTaskExecutor; import org.jabref.logic.l10n.Localization; import org.jabref.logic.search.SearchQuery; -import org.jabref.logic.search.SearchQueryHighlightObservable; import org.jabref.model.entry.Author; import org.jabref.preferences.SearchPreferences; @@ -73,7 +72,6 @@ public class GlobalSearchBar extends HBox { private final ToggleButton regularExp; private final Button searchModeButton = new Button(); private final Label currentResults = new Label(""); - private final SearchQueryHighlightObservable searchQueryHighlightObservable = new SearchQueryHighlightObservable(); private SearchDisplayMode searchDisplayMode; public GlobalSearchBar(JabRefFrame frame) { @@ -149,6 +147,16 @@ public GlobalSearchBar(JabRefFrame frame) { currentResults); this.setAlignment(Pos.CENTER_LEFT); + + //TODO Needs fixing, not yet working correctly + EasyBind.subscribe(Globals.stateManager.searchResultSizeProperty(), matchedResults -> { + + Globals.stateManager.activeSearchQueryProperty().getValue().ifPresent(searchQuery -> { + updateResults(matchedResults.intValue(), SearchDescribers.getSearchDescriberFor(searchQuery).getDescription(), + searchQuery.isGrammarBasedSearch()); + }); + + }); } private void toggleSearchModeAndSearch() { @@ -186,7 +194,6 @@ public void focus() { private void clearSearch() { currentResults.setText(""); searchField.setText(""); - Globals.stateManager.resetSearchQueryHighlightObservable(); Globals.stateManager.clearSearchQuery(); } @@ -209,18 +216,11 @@ public void performSearch() { Globals.stateManager.setSearchQuery(searchQuery); - updateResults(Globals.stateManager.searchResultSizeProperty().get(), - SearchDescribers.getSearchDescriberFor(searchQuery).getDescription(), - searchQuery.isGrammarBasedSearch()); - - } private void informUserAboutInvalidSearchQuery() { searchField.pseudoClassStateChanged(CLASS_NO_RESULTS, true); - searchQueryHighlightObservable.reset(); - Globals.stateManager.clearSearchQuery(); String illegalSearch = Localization.lang("Search failed: illegal search expression"); diff --git a/src/main/java/org/jabref/logic/search/SearchQuery.java b/src/main/java/org/jabref/logic/search/SearchQuery.java index 12b0639d6da..e6e35486767 100644 --- a/src/main/java/org/jabref/logic/search/SearchQuery.java +++ b/src/main/java/org/jabref/logic/search/SearchQuery.java @@ -3,6 +3,9 @@ import java.util.Collections; import java.util.List; import java.util.Objects; +import java.util.Optional; +import java.util.StringJoiner; +import java.util.regex.Pattern; import org.jabref.logic.l10n.Localization; import org.jabref.model.entry.BibEntry; @@ -119,6 +122,28 @@ public List getSearchWords() { } } + // Returns a regular expression pattern in the form (w1)|(w2)| ... wi are escaped if no regular expression search is enabled + public Optional getPatternForWords() { + List words = getSearchWords(); + + if ((words == null) || words.isEmpty() || words.get(0).isEmpty()) { + return Optional.empty(); + } + + // compile the words to a regular expression in the form (w1)|(w2)|(w3) + StringJoiner joiner = new StringJoiner(")|(", "(", ")"); + for (String word : words) { + joiner.add(regularExpression ? word : Pattern.quote(word)); + } + String searchPattern = joiner.toString(); + + if (caseSensitive) { + return Optional.of(Pattern.compile(searchPattern)); + } else { + return Optional.of(Pattern.compile(searchPattern, Pattern.CASE_INSENSITIVE)); + } + } + public SearchRule getRule() { return rule; } diff --git a/src/main/java/org/jabref/logic/search/SearchQueryHighlightListener.java b/src/main/java/org/jabref/logic/search/SearchQueryHighlightListener.java deleted file mode 100644 index 5f88be0ae0a..00000000000 --- a/src/main/java/org/jabref/logic/search/SearchQueryHighlightListener.java +++ /dev/null @@ -1,20 +0,0 @@ -package org.jabref.logic.search; - -import java.util.Optional; -import java.util.regex.Pattern; - -import com.google.common.eventbus.Subscribe; - -/** - * Every Listener that wants to receive events from a search needs to - * implement this interface - */ -@FunctionalInterface -public interface SearchQueryHighlightListener { - - /** - * Pattern with which one can determine what to highlight - */ - @Subscribe - void highlightPattern(Optional highlightPattern); -} diff --git a/src/main/java/org/jabref/logic/search/SearchQueryHighlightObservable.java b/src/main/java/org/jabref/logic/search/SearchQueryHighlightObservable.java deleted file mode 100644 index 1665e1b3294..00000000000 --- a/src/main/java/org/jabref/logic/search/SearchQueryHighlightObservable.java +++ /dev/null @@ -1,85 +0,0 @@ -package org.jabref.logic.search; - -import java.util.List; -import java.util.Objects; -import java.util.Optional; -import java.util.StringJoiner; -import java.util.regex.Pattern; - -import com.google.common.eventbus.EventBus; - -public class SearchQueryHighlightObservable { - - private final EventBus eventBus = new EventBus(); - - private Optional pattern = Optional.empty(); - - /** - * Adds a SearchQueryHighlightListener to the search bar. The added listener is immediately informed about the current search. - * Subscribers will be notified about searches. - * - * @param newListener SearchQueryHighlightListener to be added - */ - public void addSearchListener(SearchQueryHighlightListener newListener) { - Objects.requireNonNull(newListener); - - eventBus.register(newListener); - newListener.highlightPattern(pattern); - } - - public void removeSearchListener(SearchQueryHighlightListener listener) { - Objects.requireNonNull(listener); - - try { - eventBus.unregister(listener); - } catch (IllegalArgumentException e) { - // occurs if the event source has not been registered, should not prevent shutdown - } - } - /** - * Fires an event if a search was started (or cleared) - * - * @param searchQuery the search query - */ - - public void fireSearchlistenerEvent(SearchQuery searchQuery) { - Objects.requireNonNull(searchQuery); - - // Parse the search string to words - pattern = getPatternForWords(searchQuery.getSearchWords(), searchQuery.isRegularExpression(), - searchQuery.isCaseSensitive()); - - update(); - } - - public void reset() { - pattern = Optional.empty(); - update(); - } - - private void update() { - // Fire an event for every listener - eventBus.post(pattern); - } - - // Returns a regular expression pattern in the form (w1)|(w2)| ... wi are escaped if no regular expression search is enabled - public static Optional getPatternForWords(List words, boolean useRegex, boolean isCaseSensitive) { - if ((words == null) || words.isEmpty() || words.get(0).isEmpty()) { - return Optional.empty(); - } - - // compile the words to a regular expression in the form (w1)|(w2)|(w3) - StringJoiner joiner = new StringJoiner(")|(", "(", ")"); - for (String word : words) { - joiner.add(useRegex ? word : Pattern.quote(word)); - } - String searchPattern = joiner.toString(); - - if (isCaseSensitive) { - return Optional.of(Pattern.compile(searchPattern)); - } else { - return Optional.of(Pattern.compile(searchPattern, Pattern.CASE_INSENSITIVE)); - } - } - -} diff --git a/src/test/java/org/jabref/logic/search/SearchQueryHighlightObservableTest.java b/src/test/java/org/jabref/logic/search/SearchQueryHighlightObservableTest.java deleted file mode 100644 index fbe998e8a85..00000000000 --- a/src/test/java/org/jabref/logic/search/SearchQueryHighlightObservableTest.java +++ /dev/null @@ -1,59 +0,0 @@ -package org.jabref.logic.search; - -import java.util.Optional; -import java.util.regex.Pattern; - -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import org.mockito.ArgumentCaptor; -import org.mockito.Captor; -import org.mockito.Mock; -import org.mockito.MockitoAnnotations; - -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.mockito.Mockito.atLeastOnce; -import static org.mockito.Mockito.verify; - -class SearchQueryHighlightObservableTest { - - @Captor ArgumentCaptor> captor; - @Mock private SearchQueryHighlightListener listener; - private SearchQueryHighlightObservable observable; - - @BeforeEach - void setUp() throws Exception { - observable = new SearchQueryHighlightObservable(); - MockitoAnnotations.initMocks(this); - } - - @Test - void addSearchListenerNotifiesListenerAboutPreviousPattern() throws Exception { - observable.fireSearchlistenerEvent(new SearchQuery("test", false, false)); - - observable.addSearchListener(listener); - - verify(listener).highlightPattern(captor.capture()); - assertEquals("(\\Qtest\\E)", captor.getValue().get().pattern()); - } - - @Test - void addSearchListenerNotifiesRegisteredListener() throws Exception { - observable.addSearchListener(listener); - - observable.fireSearchlistenerEvent(new SearchQuery("test", false, false)); - - verify(listener, atLeastOnce()).highlightPattern(captor.capture()); - assertEquals("(\\Qtest\\E)", captor.getValue().get().pattern()); - } - - @Test - void addSearchListenerNotifiesRegisteredListenerAboutGrammarBasedSearches() throws Exception { - observable.addSearchListener(listener); - - observable.fireSearchlistenerEvent(new SearchQuery("author=harrer", false, false)); - - verify(listener, atLeastOnce()).highlightPattern(captor.capture()); - // TODO: We would expect "harrer" here - assertEquals("(\\Qauthor=harrer\\E)", captor.getValue().get().pattern()); - } -} diff --git a/src/test/java/org/jabref/logic/search/SearchQueryTest.java b/src/test/java/org/jabref/logic/search/SearchQueryTest.java index e4463f75155..34593c55d85 100644 --- a/src/test/java/org/jabref/logic/search/SearchQueryTest.java +++ b/src/test/java/org/jabref/logic/search/SearchQueryTest.java @@ -1,5 +1,8 @@ package org.jabref.logic.search; +import java.util.Optional; +import java.util.regex.Pattern; + import org.jabref.model.entry.BibEntry; import org.jabref.model.entry.BibtexEntryTypes; import org.jabref.model.entry.FieldName; @@ -189,7 +192,15 @@ public void testSimpleTerm() { String query = "progress"; SearchQuery result = new SearchQuery(query, false, false); - assertFalse(result.isGrammarBasedSearch()); } + + @Test + public void testGetPattern() { + String query = "progress"; + SearchQuery result = new SearchQuery(query, false, false); + Pattern pattern = Pattern.compile("(\\Qprogress\\E)"); + //We can't directly compare the pattern objects + assertEquals(Optional.of(pattern.toString()), result.getPatternForWords().map(Pattern::toString)); + } }