-
-
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
+191
−1
Merged
Changes from all 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
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
82 changes: 82 additions & 0 deletions
82
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,82 @@ | ||
| package org.jabref.gui.groups; | ||
|
|
||
| import org.jabref.logic.search.query.GroupNameFilterVisitor; | ||
|
|
||
| import org.junit.jupiter.params.ParameterizedTest; | ||
| import org.junit.jupiter.params.provider.CsvSource; | ||
|
|
||
| import static org.junit.jupiter.api.Assertions.assertEquals; | ||
|
|
||
| class GroupNodeViewModelFilterTest { | ||
|
|
||
| @ParameterizedTest | ||
| @CsvSource({ | ||
| "machine, machine learning, true", | ||
| "learning, machine learning, true", | ||
| "machine learning, machine learning, true", | ||
| "Neural Networks, machine learning, false", | ||
| "test group, test, true" | ||
| }) | ||
| void spaceImpliesOr(String groupName, String query, boolean expected) { | ||
| assertEquals(expected, GroupNameFilterVisitor.matches(groupName, query)); | ||
| } | ||
|
|
||
| @ParameterizedTest | ||
| @CsvSource({ | ||
| "machine neural, machine AND neural, true", | ||
| "machine, machine AND neural, false", | ||
| "neural, machine AND neural, false" | ||
| }) | ||
| void explicitAndRequiresBothWords(String groupName, String query, boolean expected) { | ||
| assertEquals(expected, GroupNameFilterVisitor.matches(groupName, query)); | ||
| } | ||
|
|
||
| @ParameterizedTest | ||
| @CsvSource({ | ||
| "machine, machine OR neural, true", | ||
| "neural, machine OR neural, true", | ||
| "unrelated, machine OR neural, false" | ||
| }) | ||
| void explicitOrWorks(String groupName, String query, boolean expected) { | ||
| assertEquals(expected, GroupNameFilterVisitor.matches(groupName, query)); | ||
| } | ||
|
|
||
| @ParameterizedTest | ||
| @CsvSource({ | ||
| "machine, NOT machine, false", | ||
| "learning, NOT machine, true" | ||
| }) | ||
| void notWorks(String groupName, String query, boolean expected) { | ||
| assertEquals(expected, GroupNameFilterVisitor.matches(groupName, query)); | ||
| } | ||
|
|
||
| @ParameterizedTest | ||
| @CsvSource({ | ||
| "Machine Learning, machine, true", | ||
| "machine learning, MACHINE, true" | ||
| }) | ||
| void caseInsensitiveMatch(String groupName, String query, boolean expected) { | ||
| assertEquals(expected, GroupNameFilterVisitor.matches(groupName, query)); | ||
| } | ||
|
|
||
| @ParameterizedTest | ||
| @CsvSource({ | ||
| "Deep Learning, (deep OR neural) NOT vision, true", | ||
| "Neural Networks, (deep OR neural) NOT vision, true", | ||
| "Computer Vision, (deep OR neural) NOT vision, false", | ||
| "Machine Learning, (deep OR neural) NOT vision, false" | ||
| }) | ||
| void parenthesesWithNotWork(String groupName, String query, boolean expected) { | ||
| assertEquals(expected, GroupNameFilterVisitor.matches(groupName, query)); | ||
| } | ||
|
|
||
| @ParameterizedTest | ||
| @CsvSource({ | ||
| "Computer Vision, (machine OR computer) NOT learning, true", | ||
| "Machine Learning, (machine OR computer) NOT learning, false", | ||
| "Neural Networks, (machine OR computer) NOT learning, false" | ||
| }) | ||
| void complexExpressionWithNotLearning(String groupName, String query, boolean expected) { | ||
| assertEquals(expected, GroupNameFilterVisitor.matches(groupName, query)); | ||
| } | ||
| } |
104 changes: 104 additions & 0 deletions
104
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,104 @@ | ||
| package org.jabref.logic.search.query; | ||
|
|
||
| import java.util.Locale; | ||
|
|
||
| import org.jabref.logic.util.strings.StringUtil; | ||
| import org.jabref.model.search.query.SearchQuery; | ||
| import org.jabref.search.SearchBaseVisitor; | ||
| import org.jabref.search.SearchParser; | ||
|
|
||
| import org.antlr.v4.runtime.misc.ParseCancellationException; | ||
|
|
||
| /// 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. | ||
| public class GroupNameFilterVisitor extends SearchBaseVisitor<Boolean> { | ||
|
|
||
| private final String groupName; | ||
|
|
||
| public GroupNameFilterVisitor(String groupName) { | ||
| // Lowercase for case-insensitive matching: typing "machine" matches "Machine Learning" | ||
| this.groupName = groupName.toLowerCase(Locale.ROOT); | ||
| } | ||
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
|
||
| // Determine whether this is a simple term or a fielded/operator comparison. | ||
| // If the full node text differs from the searchValue text, we assume the | ||
| // presence of a field and/or operator (e.g., "name != learning"). | ||
| String fullText = ctx.getText(); | ||
| String valueText = ctx.searchValue().getText(); | ||
|
|
||
| if (!fullText.equals(valueText)) { | ||
| // Fielded/operator comparisons are not supported by this visitor. | ||
| // Fall back to treating the entire comparison as a plain-text term, | ||
| // to avoid misinterpreting operators like "!=" as a positive match. | ||
| String plain = fullText.toLowerCase(Locale.ROOT); | ||
| return groupName.contains(plain); | ||
| } | ||
|
|
||
| String term = SearchQueryConversion.unescapeSearchValue(ctx.searchValue()).toLowerCase(Locale.ROOT); | ||
| return groupName.contains(term); | ||
| } | ||
|
|
||
| /// Implemented a static method that checks whether the group name matches the given query string. | ||
| public static boolean matches(String groupName, String query) { | ||
| if (StringUtil.isBlank(query)) { | ||
| return true; | ||
| } | ||
| try { | ||
| SearchParser.StartContext ctx = SearchQuery.getStartContext(query); | ||
| return new GroupNameFilterVisitor(groupName).visit(ctx); | ||
| } catch (ParseCancellationException e) { | ||
| return StringUtil.containsIgnoreCase(groupName, query); | ||
| } | ||
| } | ||
| } | ||
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.