Skip to content

Conversation

turhantolgaunal
Copy link
Contributor

@turhantolgaunal turhantolgaunal commented Sep 1, 2025

Followup to #13691

Updated the websearch field to be highlighted in accordance with the ANTLR based parser, rather than the old lucene one.

Steps to test

Writing a query to the web search field should turn it red if the query doesn't obey the search syntax rules.

Mandatory checks

int line = offendingToken.getLine();
int charPositionInLine = offendingToken.getCharPositionInLine() + 1;

String errorMessage = "Invalid syntax at line %d, position %d".formatted(line, charPositionInLine);
Copy link

Choose a reason for hiding this comment

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

The error message string is not using sentence case as required for UI text. It should start with a capital letter and maintain consistent casing with other UI messages.

@@ -85,18 +87,31 @@ public WebSearchPaneViewModel(GuiPreferences preferences, DialogService dialogSe
}

try {
parser.parse(queryText, NO_EXPLICIT_FIELD);
SearchLexer lexer = new SearchLexer(CharStreams.fromString(queryText));
lexer.removeErrorListeners(); // no infos on file system
Copy link

Choose a reason for hiding this comment

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

Comment provides no additional information beyond what is obvious from the code itself. Such trivial comments should be removed according to the commenting guidelines.

@turhantolgaunal turhantolgaunal marked this pull request as ready for review September 4, 2025 17:38
int charPositionInLine = offendingToken.getCharPositionInLine() + 1;

String errorMessage = "Invalid syntax at line %d, position %d".formatted(line, charPositionInLine);
return ValidationMessage.error(Localization.lang(errorMessage));
Copy link
Member

Choose a reason for hiding this comment

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

Please reuse the existing l10n: Localization.lang("Invalid query element '%0' at position %1,line, charPositioninLIne)...

@koppor koppor mentioned this pull request Sep 8, 2025
5 tasks
@Test
void queryConsistingOfInvalidDOIIsValid() {
viewModel.queryProperty().setValue("101.1007/JHEP02(2023)082");
// There is currently no interpretation of nearly-valid identifiers, therefore, this is concidered as "regular" search term
Copy link

Choose a reason for hiding this comment

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

Misspelled word in comment ('concidered' should be 'considered'). Variable names and comments should be correctly spelled to maintain code quality.

@koppor koppor added the status: awaiting-second-review For non-trivial changes label Sep 8, 2025
viewModel.queryProperty().setValue("101.1007/JHEP02(2023)082");
assertFalse(viewModel.queryValidationStatus().validProperty().getValue());
// There is currently no interpretation of nearly-valid identifiers, therefore, this is concidered as "regular" search term
Copy link

Choose a reason for hiding this comment

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

The comment merely restates what the code does and contains a spelling error ('concidered'). Comments should provide additional information not obvious from the code.

@Siedlerchr Siedlerchr added this pull request to the merge queue Sep 8, 2025
Merged via the queue into JabRef:main with commit 0eb694b Sep 8, 2025
48 of 55 checks passed
Siedlerchr added a commit that referenced this pull request Sep 8, 2025
* upstream/main: (54 commits)
  Split relativizeSymlinks parameterized tests in separate tests (#13782)
  Update the search syntax highlight for web search (#13801)
  Chore(deps): Bump ai.djl:bom from 0.33.0 to 0.34.0 in /versions (#13833)
  Fix typos in CHANGELOG.md (#13826)
  Chore(deps): Bump com.konghq:unirest-modules-gson in /versions (#13831)
  Chore(deps): Bump org.gradlex:extra-java-module-info in /build-logic (#13830)
  Chore(deps): Bump org.apache.logging.log4j:log4j-to-slf4j in /versions (#13832)
  Chore(deps): Bump io.zonky.test.postgres:embedded-postgres-binaries-bom (#13834)
  Chore(deps): Bump jablib/src/main/resources/csl-locales (#13829)
  Chore(deps): Bump jablib/src/main/resources/csl-styles (#13827)
  Chore(deps): Bump jablib/src/main/abbrv.jabref.org (#13828)
  add: CAYW endpoint formats (#13785)
  New Crowdin updates (#13823)
  chore(deps): update dependency org.kohsuke:github-api to v2.0-rc.5 (#13822)
  Add support for automatic ICORE conference ranking lookup [#13476] (#13699)
  New Crowdin updates (#13820)
  Initialize search bar auto-completion with real database context (no tab switch needed) (#13816)
  Fixes #13274: Allow cygwin-paths on Windows (#13297)
  Refine "REDACTED" replacement of API key value in web fetcher search URL (#13814)
  changed ISSNCleanup into NormalizeIssn, refactored respective tests #13748 (#13767)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: awaiting-second-review For non-trivial changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants