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.

Sign up for GitHub

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

Jump to bottom

Feature/add ui for query parsing #6805

Merged
merged 2 commits into from
Sep 1, 2020
Merged

Feature/add ui for query parsing #6805

merged 2 commits into from
Sep 1, 2020

Conversation

DominikVoigt
Copy link
Contributor

@DominikVoigt DominikVoigt commented Aug 28, 2020 •
edited
Loading

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
Copy link
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
Copy link
Contributor Author

DominikVoigt commented Aug 30, 2020 •
edited
Loading

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
Copy link
Member

koppor commented Aug 31, 2020

This refs koppor#341.

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

koppor commented Aug 31, 2020

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
Copy link
Member

@calixtus calixtus left a comment

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

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);");
Copy link
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
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
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