Skip to content

Commit

Permalink
Feature/add ui for query parsing (#6805)
Browse files Browse the repository at this point in the history
* Add query syntax validity feedback to UI
Make QueryParser handle query parsing issues more robust
Make default implementation of performComplexSearch include all fields

Signed-off-by: Dominik Voigt <dominik.ingo.voigt@gmail.com>

* Make UI use advanced formation

Signed-off-by: Dominik Voigt <dominik.ingo.voigt@gmail.com>
  • Loading branch information
DominikVoigt committed Sep 1, 2020
1 parent c74c557 commit 5d6c2ee
Show file tree
Hide file tree
Showing 8 changed files with 117 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,23 @@ For simplicitly, and lack of universal capabilities across fetchers, only basic
* `year` (for single year)
* `year-range` (for range e.g. `year-range:2012-2015`)
* The `journal`, `year`, and `year-range` fields should only be populated once in each query
* The `year` and `year-range` fields are mutually exclusive
* Example:
* `author:"Igor Steinmacher" author:"Christoph Treude" year:2017` will be converted to
* `author:"Igor Steinmacher" AND author:"Christoph Treude" AND year:2017`

The supported syntax can be expressed in EBNF as follows:

Query := {Clause} \
Clause:= \[Field\] Term \
Field := author: | title: | journal: | year: | year-range: | default:\
Term := Word | Phrase \

Word can be derived to any series of non-whitespace characters.
Phrases are multiple words wrapped in quotes and may contain white-space characters within the quotes.\
Note: Even though this EBNF syntactically allows the creation of queries with year and year-range fields,
such a query does not make sense semantically and therefore will not be executed.

### Positive Consequences

* Already tested
Expand Down
8 changes: 8 additions & 0 deletions src/main/java/org/jabref/gui/Base.css
Original file line number Diff line number Diff line change
Expand Up @@ -1173,3 +1173,11 @@ We want to have a look that matches our icons in the tool-bar */
TextFlow * {
-fx-fill: -fx-text-background-color;
}

.searchBar:invalid {
-fx-background-color: rgba(240, 128, 128, 0.5);
}

.searchBar:unsupported {
-fx-background-color: rgba(255, 159, 67, 0.5);
}
11 changes: 10 additions & 1 deletion src/main/java/org/jabref/gui/importer/fetcher/WebSearchPane.java
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,16 @@ protected Node createContentPane() {

// Create text field for query input
TextField query = SearchTextField.create();
query.setOnAction(event -> viewModel.search());
query.getStyleClass().add("searchBar");
query.textProperty().addListener((observable, oldValue, newValue) -> viewModel.validateQueryStringAndGiveColorFeedback(query, newValue));
query.focusedProperty().addListener((observable, oldValue, newValue) -> {
if (newValue) {
viewModel.validateQueryStringAndGiveColorFeedback(query, query.getText());
} else {
viewModel.setPseudoClassToValid(query);
}
});

viewModel.queryProperty().bind(query.textProperty());

// Create button that triggers search
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package org.jabref.gui.importer.fetcher;

import java.util.Optional;
import java.util.SortedSet;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import javafx.beans.property.ListProperty;
import javafx.beans.property.ObjectProperty;
Expand All @@ -10,15 +13,20 @@
import javafx.beans.property.StringProperty;
import javafx.collections.FXCollections;
import javafx.collections.ObservableList;
import javafx.css.PseudoClass;
import javafx.scene.control.TextField;
import javafx.scene.control.Tooltip;

import org.jabref.gui.DialogService;
import org.jabref.gui.JabRefFrame;
import org.jabref.gui.importer.ImportEntriesDialog;
import org.jabref.gui.util.BackgroundTask;
import org.jabref.logic.importer.ImportFormatPreferences;
import org.jabref.logic.importer.ParserResult;
import org.jabref.logic.importer.QueryParser;
import org.jabref.logic.importer.SearchBasedFetcher;
import org.jabref.logic.importer.WebFetchers;
import org.jabref.logic.importer.fetcher.ComplexSearchQuery;
import org.jabref.logic.l10n.Localization;
import org.jabref.model.strings.StringUtil;
import org.jabref.preferences.JabRefPreferences;
Expand All @@ -32,6 +40,8 @@ public class WebSearchPaneViewModel {
private final StringProperty query = new SimpleStringProperty();
private final JabRefFrame frame;
private final DialogService dialogService;
private final Pattern queryPattern;
private final Pattern laxQueryPattern;

public WebSearchPaneViewModel(ImportFormatPreferences importPreferences, JabRefFrame frame, JabRefPreferences preferences, DialogService dialogService) {
// TODO: Rework so that we don't rely on JabRefFrame and not the complete preferences
Expand All @@ -52,6 +62,13 @@ public WebSearchPaneViewModel(ImportFormatPreferences importPreferences, JabRefF
int newIndex = fetchers.indexOf(newFetcher);
preferences.putInt(JabRefPreferences.SELECTED_FETCHER_INDEX, newIndex);
});

String allowedFields = "((author|journal|title|year|year-range):\\s?)?";
// Either a single word, or a phrase with quotes, or a year-range
String allowedTermText = "(((\\d{4}-\\d{4})|(\\w+)|(\"\\w+[^\"]*\"))\\s?)+";
queryPattern = Pattern.compile("^(" + allowedFields + allowedTermText + ")+$");
String laxFields = "(\\w+:\\s?)?";
laxQueryPattern = Pattern.compile("^(" + laxFields + allowedTermText + ")+$");
}

public ObservableList<SearchBasedFetcher> getFetchers() {
Expand Down Expand Up @@ -91,13 +108,57 @@ public void search() {

SearchBasedFetcher activeFetcher = getSelectedFetcher();

BackgroundTask<ParserResult> task = BackgroundTask.wrap(() -> new ParserResult(activeFetcher.performSearch(getQuery().trim())))
.withInitialMessage(Localization.lang("Processing %0", getQuery()));

BackgroundTask<ParserResult> task;
QueryParser queryParser = new QueryParser();
Optional<ComplexSearchQuery> generatedQuery = queryParser.parseQueryStringIntoComplexQuery(getQuery());
if (generatedQuery.isPresent()) {
task = BackgroundTask.wrap(() -> new ParserResult(activeFetcher.performComplexSearch(generatedQuery.get())))
.withInitialMessage(Localization.lang("Processing %0", getQuery()));
} else {
task = BackgroundTask.wrap(() -> new ParserResult(activeFetcher.performSearch(getQuery().trim())))
.withInitialMessage(Localization.lang("Processing %0", getQuery()));
}
task.onFailure(dialogService::showErrorDialogAndWait);

ImportEntriesDialog dialog = new ImportEntriesDialog(frame.getCurrentBasePanel().getBibDatabaseContext(), task);
dialog.setTitle(activeFetcher.getName());
dialog.showAndWait();
}

public void validateQueryStringAndGiveColorFeedback(TextField querySource, String queryString) {
Matcher queryValidation = queryPattern.matcher(queryString.strip());
if (!queryString.strip().isBlank() && !queryValidation.matches()) {
Matcher laxQueryValidation = laxQueryPattern.matcher(queryString.strip());
if (laxQueryValidation.matches()) {
setPseudoClassToUnsupported(querySource);
querySource.setTooltip(new Tooltip(Localization.lang("This query uses unsupported fields.")));
} else {
setPseudoClassToInvalid(querySource);
querySource.setTooltip(new Tooltip(Localization.lang("This query uses unsupported syntax.")));
}
} else if (containsYearAndYearRange(queryString)) {
setPseudoClassToInvalid(querySource);
querySource.setTooltip(new Tooltip(Localization.lang("The query cannot contain a year and year-range field.")));
} else {
setPseudoClassToValid(querySource);
}
}

private void setPseudoClassToUnsupported(TextField querySource) {
querySource.pseudoClassStateChanged(PseudoClass.getPseudoClass("invalid"), false);
querySource.pseudoClassStateChanged(PseudoClass.getPseudoClass("unsupported"), true);
}

public void setPseudoClassToValid(TextField querySource) {
querySource.pseudoClassStateChanged(PseudoClass.getPseudoClass("invalid"), false);
querySource.pseudoClassStateChanged(PseudoClass.getPseudoClass("unsupported"), false);
}

private void setPseudoClassToInvalid(TextField querySource) {
querySource.pseudoClassStateChanged(PseudoClass.getPseudoClass("invalid"), true);
}

private boolean containsYearAndYearRange(String queryString) {
return queryString.toLowerCase().contains("year:") && queryString.toLowerCase().contains("year-range:");
}
}
7 changes: 3 additions & 4 deletions src/main/java/org/jabref/logic/importer/QueryParser.java
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,9 @@ public Optional<ComplexSearchQuery> parseQueryStringIntoComplexQuery(String quer
luceneQuery.visit(visitor);

List<Term> sortedTerms = new ArrayList<>(terms);
sortedTerms.sort(Comparator.comparing(Term::text));
builder.terms(sortedTerms);
return Optional.of(builder.build());
} catch (QueryNodeException | IllegalStateException ex) {
sortedTerms.sort(Comparator.comparing(Term::text).reversed());
return Optional.of(ComplexSearchQuery.fromTerms(terms));
} catch (QueryNodeException | IllegalStateException | IllegalArgumentException ex) {
return Optional.empty();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,7 @@ public interface SearchBasedFetcher extends WebFetcher {
* @return a list of {@link BibEntry}, which are matched by the query (may be empty)
*/
default List<BibEntry> performComplexSearch(ComplexSearchQuery complexSearchQuery) throws FetcherException {
// Default implementation behaves as performSearch using the default field phrases as query
List<String> defaultPhrases = complexSearchQuery.getDefaultFieldPhrases();
return performSearch(String.join(" ", defaultPhrases));
// Default implementation behaves as perform search on all fields concatenated as query
return performSearch(complexSearchQuery.toString());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,19 @@ public int hashCode() {
return result;
}

@Override
public String toString() {
StringBuilder stringRepresentation = new StringBuilder();
getSingleYear().ifPresent(singleYear -> stringRepresentation.append(singleYear).append(" "));
getFromYear().ifPresent(fromYear -> stringRepresentation.append(fromYear).append(" "));
getToYear().ifPresent(toYear -> stringRepresentation.append(toYear).append(" "));
getJournal().ifPresent(journal -> stringRepresentation.append(journal).append(" "));
stringRepresentation.append(String.join(" ", getTitlePhrases()))
.append(String.join(" ", getDefaultFieldPhrases()))
.append(String.join(" ", getAuthors()));
return stringRepresentation.toString();
}

public static class ComplexSearchQueryBuilder {
private List<String> defaultFieldPhrases = new ArrayList<>();
private List<String> authors = new ArrayList<>();
Expand Down
6 changes: 4 additions & 2 deletions src/main/resources/l10n/JabRef_en.properties
Original file line number Diff line number Diff line change
Expand Up @@ -2259,14 +2259,16 @@ Reveal\ in\ file\ explorer=Reveal in file explorer
Autolink\ files=Autolink files
(\ Note\:\ Press\ return\ to\ commit\ changes\ in\ the\ table\!\ )=( Note\: Press return to commit changes in the table\! )
Reset=Reset
Reset\ entry\ types\ and\ fields\ to\ defaults=Reset entry types and fields to defaults
This\ will\ reset\ all\ entry\ types\ to\ their\ default\ values\ and\ remove\ all\ custom\ entry\ types=This will reset all entry types to their default values and remove all custom entry types
Replace\ tabs\ with\ space=Replace tabs with space
Replace\ tabs\ with\ space\ in\ the\ field\ content.=Replace tabs with space in the field content.
Remove\ redundant\ spaces=Remove redundant spaces
Replaces\ consecutive\ spaces\ with\ a\ single\ space\ in\ the\ field\ content.=Replaces consecutive spaces with a single space in the field content.
Remove\ digits=Remove digits
Removes\ digits.=Removes digits.
The\ query\ cannot\ contain\ a\ year\ and\ year-range\ field.=The query cannot contain a year and year-range field.
This\ query\ uses\ an\ unsupported\ syntax.=This query uses an unsupported syntax.
This\ search\ contains\ entries\ that\ match\ all\ specified\ terms.=This search contains entries that match all specified terms.

0 comments on commit 5d6c2ee

Please sign in to comment.