-
-
Notifications
You must be signed in to change notification settings - Fork 3.3k
Fix group filter to use OR semantics for space separated terms #15350
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.
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
Merged
calixtus
merged 35 commits into
JabRef:main
from
NishantDG-SST:fix-group-filter-or-semantics-12721
Apr 5, 2026
Merged
Changes from 2 commits
Commits
Show all changes
35 commits
Select commit
Hold shift + click to select a range
e3d7134
Fix group filter to use OR semantics for space-separated terms
NishantDG-SST fa84c44
Update CHANGELOG for #12721
NishantDG-SST 253bba2
Merge branch 'main' into fix-group-filter-or-semantics-12721
NishantDG-SST cfad844
Address review feedback: Locale.ROOT, /// javadoc, parse-once, rename…
NishantDG-SST 43705d7
added comment for lowerCase, converted Tests to parameterized tests, …
NishantDG-SST 117f846
Fixed visitComparison to fallback to plain text for fielded/operator …
NishantDG-SST 184bf78
Restore accidentally removed blank
NishantDG-SST b74ebd2
Implemented static matches method, resolved logic handeling test help…
NishantDG-SST 7f9460e
Removed wrapper matches class Utilised the static matches method dire…
NishantDG-SST 98b3f3b
ci: retrigger
NishantDG-SST 4fe94ed
ci: retrigger
NishantDG-SST 380d1bb
Merge remote-tracking branch 'upstream/main' into fix-group-filter-or…
NishantDG-SST 4dba5b7
ci: retrigger
NishantDG-SST d9e7b02
Merge remote-tracking branch 'upstream/main' into fix-group-filter-or…
NishantDG-SST f182393
Merge remote-tracking branch 'upstream/main' into fix-group-filter-or…
NishantDG-SST f0c7a12
Merge branch 'main' into fix-group-filter-or-semantics-12721
NishantDG-SST 72a8b8d
Merge remote-tracking branch 'upstream/main' into fix-group-filter-or…
NishantDG-SST e6034c9
Merge branch 'fix-group-filter-or-semantics-12721' of https://github.…
NishantDG-SST 6ed0f51
Merge branch 'main' into fix-group-filter-or-semantics-12721
NishantDG-SST 570a8eb
Merge remote-tracking branch 'upstream/main' into fix-group-filter-or…
NishantDG-SST 2cb27f5
Merge branch 'fix-group-filter-or-semantics-12721' of https://github.…
NishantDG-SST 8d2b862
Merge branch 'main' into fix-group-filter-or-semantics-12721
NishantDG-SST fc598d1
Merge remote-tracking branch 'upstream/main' into fix-group-filter-or…
NishantDG-SST 19a81cb
Merge branch 'fix-group-filter-or-semantics-12721' of https://github.…
NishantDG-SST 54c81f2
Merge remote-tracking branch 'upstream/main' into fix-group-filter-or…
NishantDG-SST 5930bba
Retrigger CI checks
NishantDG-SST 83ffe3e
Retrigger CI checks
NishantDG-SST f88e8f5
Merge remote-tracking branch 'upstream/main' into fix-group-filter-or…
NishantDG-SST 32efe4b
Merge branch 'main' into fix-group-filter-or-semantics-12721
NishantDG-SST 146a5ac
Merge remote-tracking branch 'upstream/main' into fix-group-filter-or…
NishantDG-SST 2af1f2e
Merge branch 'fix-group-filter-or-semantics-12721' of https://github.…
NishantDG-SST 1ad23ed
Merge branch 'main' into fix-group-filter-or-semantics-12721
NishantDG-SST 327b6e1
Update CHANGELOG.md
calixtus 19f9b97
Merge branch 'main' into fix-group-filter-or-semantics-12721
calixtus 36daa50
Merge branch 'main' into fix-group-filter-or-semantics-12721
calixtus File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
90 changes: 90 additions & 0 deletions
90
jabgui/src/test/java/org/jabref/gui/groups/GroupNodeViewModelFilterTest.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,90 @@ | ||
| package org.jabref.gui.groups; | ||
|
|
||
| import org.jabref.logic.search.query.GroupNameFilterVisitor; | ||
| import org.jabref.model.search.query.SearchQuery; | ||
|
|
||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| import static org.junit.jupiter.api.Assertions.assertFalse; | ||
| import static org.junit.jupiter.api.Assertions.assertTrue; | ||
|
|
||
| class GroupNodeViewModelFilterTest { | ||
|
|
||
| private boolean matches(String groupName, String query) { | ||
| if (query == null || query.isBlank()) { | ||
| return true; | ||
| } | ||
| try { | ||
| var ctx = SearchQuery.getStartContext(query); | ||
| return new GroupNameFilterVisitor(groupName).visit(ctx); | ||
| } catch (org.antlr.v4.runtime.misc.ParseCancellationException e) { | ||
| return groupName.toLowerCase().contains(query.toLowerCase()); | ||
| } | ||
| } | ||
calixtus marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| @Test | ||
| void spaceImpliesOrFirstToken() { | ||
| assertTrue(matches("machine", "machine learning")); | ||
| } | ||
|
|
||
| @Test | ||
| void spaceImpliesOrSecondToken() { | ||
| assertTrue(matches("learning", "machine learning")); | ||
| } | ||
|
|
||
| @Test | ||
| void spaceImpliesOrBothTokens() { | ||
| assertTrue(matches("machine learning", "machine learning")); | ||
| } | ||
|
|
||
| @Test | ||
| void explicitAndRequiresBothWords() { | ||
| assertFalse(matches("machine", "machine AND neural")); | ||
| assertTrue(matches("machine neural", "machine AND neural")); | ||
| } | ||
|
|
||
| @Test | ||
| void explicitOrWorks() { | ||
| assertTrue(matches("machine", "machine OR neural")); | ||
| assertTrue(matches("neural", "machine OR neural")); | ||
| assertFalse(matches("unrelated", "machine OR neural")); | ||
| } | ||
|
|
||
| @Test | ||
| void notWorks() { | ||
| assertFalse(matches("machine", "NOT machine")); | ||
| assertTrue(matches("learning", "NOT machine")); | ||
| } | ||
|
|
||
| @Test | ||
| void blankQueryMatchesAll() { | ||
| assertTrue(matches("anything", "")); | ||
| assertTrue(matches("anything", " ")); | ||
| } | ||
|
|
||
| @Test | ||
| void caseInsensitiveMatch() { | ||
| assertTrue(matches("Machine Learning", "machine")); | ||
| assertTrue(matches("machine learning", "MACHINE")); | ||
| } | ||
|
|
||
| @Test | ||
| void malformedQueryFallsBackToContains() { | ||
| assertTrue(matches("test group", "test")); | ||
|
||
| } | ||
|
|
||
| @Test | ||
| void parenthesesWithNotWork() { | ||
| assertTrue(matches("Deep Learning", "(deep OR neural) NOT vision")); | ||
| assertTrue(matches("Neural Networks", "(deep OR neural) NOT vision")); | ||
| assertFalse(matches("Computer Vision", "(deep OR neural) NOT vision")); | ||
| assertFalse(matches("Machine Learning", "(deep OR neural) NOT vision")); | ||
| } | ||
|
|
||
| @Test | ||
| void complexExpressionWithNotLearning() { | ||
| assertTrue(matches("Computer Vision", "(machine OR computer) NOT learning")); | ||
| assertFalse(matches("Machine Learning", "(machine OR computer) NOT learning")); | ||
| assertFalse(matches("Neural Networks", "(machine OR computer) NOT learning")); | ||
| } | ||
calixtus marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
73 changes: 73 additions & 0 deletions
73
jablib/src/main/java/org/jabref/logic/search/query/GroupNameFilterVisitor.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| package org.jabref.logic.search.query; | ||
|
|
||
| import org.jabref.search.SearchBaseVisitor; | ||
| import org.jabref.search.SearchParser; | ||
|
|
||
| /** | ||
| * Evaluates a Search.g4 parse tree against a group display name string. | ||
| * Key behavioral difference from {@link SearchQueryVisitor}: | ||
| * {@code visitImplicitAndExpression} uses OR semantics (anyMatch) instead of AND, | ||
| * so space-separated bare terms like "machine learning" match any group containing | ||
| * either word. Explicit AND/OR/NOT and parentheses work as expected. | ||
| */ | ||
qodo-free-for-open-source-projects[bot] marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
calixtus marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| public class GroupNameFilterVisitor extends SearchBaseVisitor<Boolean> { | ||
|
|
||
| private final String groupName; | ||
|
|
||
| public GroupNameFilterVisitor(String groupName) { | ||
| this.groupName = groupName.toLowerCase(); | ||
| } | ||
NishantDG-SST marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| @Override | ||
| public Boolean visitStart(SearchParser.StartContext ctx) { | ||
| if (ctx.andExpression() == null) { | ||
| return true; | ||
| } | ||
| return visit(ctx.andExpression()); | ||
| } | ||
|
|
||
| @Override | ||
| public Boolean visitImplicitAndExpression(SearchParser.ImplicitAndExpressionContext ctx) { | ||
| if (ctx.expression().size() == 1) { | ||
| return visit(ctx.expression().getFirst()); | ||
| } | ||
| boolean allSimpleTerm = ctx.expression().stream() | ||
| .allMatch(e -> e instanceof SearchParser.ComparisonExpressionContext); | ||
| if (allSimpleTerm) { | ||
| return ctx.expression().stream().anyMatch(this::visit); | ||
| } else { | ||
| return ctx.expression().stream().allMatch(this::visit); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public Boolean visitBinaryExpression(SearchParser.BinaryExpressionContext ctx) { | ||
| boolean left = visit(ctx.left); | ||
| boolean right = visit(ctx.right); | ||
| return ctx.bin_op.getType() == SearchParser.AND | ||
| ? left && right | ||
| : left || right; | ||
| } | ||
|
|
||
| @Override | ||
| public Boolean visitNegatedExpression(SearchParser.NegatedExpressionContext ctx) { | ||
| return !visit(ctx.expression()); | ||
| } | ||
|
|
||
| @Override | ||
| public Boolean visitParenExpression(SearchParser.ParenExpressionContext ctx) { | ||
| return visit(ctx.andExpression()); | ||
| } | ||
|
|
||
| @Override | ||
| public Boolean visitComparisonExpression(SearchParser.ComparisonExpressionContext ctx) { | ||
| return visit(ctx.comparison()); | ||
| } | ||
|
|
||
| @Override | ||
| public Boolean visitComparison(SearchParser.ComparisonContext ctx) { | ||
calixtus marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| String term = SearchQueryConversion.unescapeSearchValue(ctx.searchValue()).toLowerCase(); | ||
| return groupName.contains(term); | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.