Skip to content

Migrate fetchers to Search.g4 ANTLR parser. #13691

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
Merged
merged 41 commits into from
Aug 25, 2025
Merged

Migrate fetchers to Search.g4 ANTLR parser. #13691

merged 41 commits into from
Aug 25, 2025
+672 −468

Conversation

turhantolgaunal
Copy link
Contributor

@turhantolgaunal turhantolgaunal commented Aug 13, 2025 •
edited by calixtus
Loading

Closes #13607

  • Changed inherited fetcher classes to use ANTLR generated classes instead of lucene libraries.
  • Changed ACMPortalFetcher.java logic for transforming the parsed syntax to URL

TO DO:

  • Replace the logic in other fetcher classes to transform the parsed nodes into URL calls.
  • Modify other fetcher classes to correctly override changed inherited functions.

Steps to test

Using the Search.g4 syntax for searching on the web with different options.

Documentation issue JabRef/user-documentation#584

Mandatory checks

  • I own the copyright of the code submitted and I license it under the MIT license
  • Change in CHANGELOG.md described in a way that is understandable for the average user (if change is visible to the user)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • [/] Screenshots added in PR description (if change is visible to the user)
  • Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
  • Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

@turhantolgaunal
pr: migrate fetchers to Search.g4 ANTLR parser.
b02fbde 
-Changed inherited fetcher classes to use ANTLR generated classes instead of lucene libraries.
- Changed ACMPortalFetcher.java logic for transforming the parsed syntax to URL
@calixtus
Copy link
Member

Great start, to me it looks like your on the right track.

@calixtus calixtus changed the title pr: migrate fetchers to Search.g4 ANTLR parser. Migrate fetchers to Search.g4 ANTLR parser. Aug 14, 2025
@calixtus calixtus added dev: code-quality Issues related to code or architecture decisions component: fetcher labels Aug 14, 2025
@turhantolgaunal
pr: migrate fetchers to Search.g4 ANTLR parser.
9892bec 
- Changed ACMPortalFetcherTest unit test code to use Search.g4 generated classes instead of Lucene
- Removed trivial comment from ACMPortalFetcher
@turhantolgaunal
pr: migrate fetchers to Search.g4 ANTLR parser.
d802521 
- Changed AbstractQueryTransformer methods to obey Search.g4 parser rules
- Modified ACMPortalFetcher to use the changed transformer logic
@turhantolgaunal
Added a new interface for search nodes in order to parse operators co…
a645b38 
…rrectly
@turhantolgaunal
Added 2 new node types and changed SearchQueryNode to implement the n…
44853c3 
…ew interface
@turhantolgaunal
Created a new visitor class for parsing the search syntax and modifie…
a643d22 
…d the search based fetcher classes to use it
@turhantolgaunal
Updated AbstractQueryTransformer to be more in line with the older co…
c890e92 
…de while still being compatible with Search.g4 parser
@turhantolgaunal
Updated ACMPortalFetcher to use the new parser and node logic
954e1a4
@turhantolgaunal
Updated ArxivPortalFetcher to use the new parser and node logic
3977901
trag-bot[bot]
trag-bot bot reviewed Aug 17, 2025
View reviewed changes
@turhantolgaunal turhantolgaunal marked this pull request as ready for review August 24, 2025 18:21
@calixtus calixtus added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Aug 24, 2025
calixtus
calixtus reviewed Aug 24, 2025
View reviewed changes
Siedlerchr
Siedlerchr previously approved these changes Aug 25, 2025
View reviewed changes
Copy link
Member

@Siedlerchr Siedlerchr left a comment

Choose a reason for hiding this comment

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

cool thanks for the work on this!

@Siedlerchr
Copy link
Member

What about the todos in the PR, are they addressed? Do you plan to address them now or in a follow up?

@trag-bot Trag bot
Copy link

trag-bot bot commented Aug 25, 2025

@trag-bot didn't find any issues in the code! ✅✨

@calixtus
Copy link
Member

Documentation issue is JabRef/user-documentation#584

@calixtus
Copy link
Member

Tested it, works fine. ❤️

calixtus
calixtus approved these changes Aug 25, 2025
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.

Thank you very much for this PR. Yes it was a lot of work and a hell of a ride through the search subsystem, but I hope it was worth it for you. It is a great addition to our codebase and we appreciate it very much!

@calixtus calixtus added this pull request to the merge queue Aug 25, 2025
Merged via the queue into JabRef:main with commit 768c86a Aug 25, 2025
44 checks passed
@github-actions github-actions bot mentioned this pull request Aug 25, 2025
Closed
@turhantolgaunal
Copy link
Contributor Author

What about the todos in the PR, are they addressed? Do you plan to address them now or in a follow up?

I had added them when I opened the pr request as a draft and when the work was incomplete, they are done.

@turhantolgaunal
Copy link
Contributor Author

Thank you very much for this PR. Yes it was a lot of work and a hell of a ride through the search subsystem, but I hope it was worth it for you. It is a great addition to our codebase and we appreciate it very much!

Thank you! I really appreciate you saying that. It was definitely a challenge but I think it gave me valuable experience.

@turhantolgaunal turhantolgaunal mentioned this pull request Aug 29, 2025
Merged
6 tasks
@koppor
Copy link
Member

koppor commented Aug 31, 2025

The task was not completely described.

The query validator on the UI seems to use a different validation.

grafik

@turhantolgaunal Do you think, you can take care of this?

@turhantolgaunal
Copy link
Contributor Author

The task was not completely described.

The query validator on the UI seems to use a different validation.
grafik

@turhantolgaunal Do you think, you can take care of this?

Ok, I will look into it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@trag-bot trag-bot[bot] trag-bot[bot] left review comments

@calixtus calixtus calixtus approved these changes

@Siedlerchr Siedlerchr Siedlerchr left review comments

Assignees
No one assigned
Labels
component: fetcher dev: code-quality Issues related to code or architecture decisions 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.

Migrate Fetcher to Search.g4
4 participants
@turhantolgaunal @calixtus @Siedlerchr @koppor