Skip to content

Feature/add ui for query parsing#6805

Merged
tobiasdiez merged 2 commits into
JabRef:masterfrom
DominikVoigt:feature/add-ui-for-query-parsing
Sep 1, 2020
Merged

Feature/add ui for query parsing#6805
tobiasdiez merged 2 commits into
JabRef:masterfrom
DominikVoigt:feature/add-ui-for-query-parsing

Conversation

@DominikVoigt

@DominikVoigt DominikVoigt commented Aug 28, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor

This PR is a follow-up of #6799 and adds the related changes to the UI code.

The search bar will be highlighted green if the supported syntax is used, and red otherwise.

Using supported syntax:
image

Missing quotation mark:
image

Using unsupported field:
image

  • Change in CHANGELOG.md described (if applicable)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (for UI changes)
  • Checked documentation: Is the information available and up to date? If not created an issue at https://github.com/JabRef/user-documentation/issues or, even better, submitted a pull request to the documentation repository.

@koppor koppor marked this pull request as draft August 28, 2020 20:56
@tobiasdiez

tobiasdiez commented Aug 28, 2020

Copy link
Copy Markdown
Member

Mhhh, do you think the toggle is really necessary? I would try to have the UI as minimal as possible. Are there negative side effects to always parse the query using lucene? If I've understood your other PR correctly, input like "Some long text: subtitle" is still parsed as one default field string, right?

@DominikVoigt DominikVoigt marked this pull request as ready for review August 30, 2020 12:01
@DominikVoigt

DominikVoigt commented Aug 30, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Mhhh, do you think the toggle is really necessary? I would try to have the UI as minimal as possible. Are there negative side effects to always parse the query using lucene? If I've understood your other PR correctly, input like "Some long text: subtitle" is still parsed as one default field string, right?

In the case of an unknown field, currently, the fielded term will be ignored.
Example: author:"Jab Ref" citation-count:29 will be mapped to "Jab Ref" in the author field.
citation-count:29 will be ignored as it does not match any field.

But this is the implemented behaviour and is independent of lucene and therefore can be adapted.

@koppor

koppor commented Aug 31, 2020

Copy link
Copy Markdown
Member

This refs https://github.com/koppor/jabref/issues/341.

@DominikVoigt DominikVoigt force-pushed the feature/add-ui-for-query-parsing branch from ebceab8 to e3075dc Compare August 31, 2020 11:53
@koppor

koppor commented Aug 31, 2020

Copy link
Copy Markdown
Member

I would really like to see the web search behaving similar to the JabRef search:

https://docs.jabref.org/finding-sorting-and-cleaning-entries/search#search-modes

  • Display "mode": normal mode and complex mode (see the coloring of the search icon) (I hope, we did not remove this functionality. If yes, please link the PR with the discussion)
  • Optional: Hover on query displays text explaining the query

@koppor koppor added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Aug 31, 2020
calixtus
calixtus requested changes Aug 31, 2020
View reviewed changes

@calixtus calixtus left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Great work. Thanks! However, found one little issue.
Greetings from JabCon2020

Comment thread src/main/java/org/jabref/gui/importer/fetcher/WebSearchPaneViewModel.java Outdated
Matcher queryValidation = queryPattern.matcher(queryString.strip());
if (queryValidation.matches()) {
if (containsYearAndYearRange(queryString)) {
querySource.setStyle("-fx-background-color: rgba(240, 128, 128, 0.5);");

@calixtus calixtus Aug 31, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Pseudo-classes could be used instead of hardcoding the colors here. See gui/util/ViewModelListCellFactory

@DominikVoigt
Add query syntax validity feedback to UI
ebcb7dc 
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>
@DominikVoigt DominikVoigt force-pushed the feature/add-ui-for-query-parsing branch from be03839 to ebcb7dc Compare September 1, 2020 10:39
@DominikVoigt
Make UI use advanced formation
6fd39c7 
Signed-off-by: Dominik Voigt <dominik.ingo.voigt@gmail.com>
@tobiasdiez tobiasdiez merged commit 5d6c2ee into JabRef:master Sep 1, 2020
Siedlerchr added a commit that referenced this pull request Sep 1, 2020
@Siedlerchr
Merge remote-tracking branch 'upstream/master' into writebackupToTemp
350f3e9 
* upstream/master:
  Feature/add ui for query parsing (#6805)
  Shared database synchronized by FocusChangedEvent (#6771)
@DominikVoigt DominikVoigt mentioned this pull request Sep 2, 2020
Merged
5 tasks
@eludif eludif mentioned this pull request Dec 31, 2020
Open
1 task
@koppor koppor mentioned this pull request Oct 2, 2024
Closed
@koppor koppor mentioned this pull request Jul 29, 2025
Closed
@koppor koppor mentioned this pull request Dec 17, 2025
Closed
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@calixtus calixtus calixtus requested changes

Assignees

No one assigned

Labels

component: search status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@DominikVoigt @tobiasdiez @koppor @calixtus