Skip to content

Fix hash warning#15596

Merged
calixtus merged 1 commit intomainfrom
fix-hash-warning
Apr 20, 2026
Merged

Fix hash warning#15596
calixtus merged 1 commit intomainfrom
fix-hash-warning

Conversation

@calixtus
Copy link
Copy Markdown
Member

Related issues and pull requests

Follow up to #15559
Fixes https://github.com/JabRef/jabref-issue-melting-pot/issues/1279
Fixes #15586

PR Description

See qodo

Steps to test

Run JabRef, write something in entry editor, see no warning about hashes

Checklist

  • I own the copyright of the code submitted and I license it under the MIT license
  • [.] I manually tested my changes in running JabRef (always required)
  • [.] I added JUnit tests for changes (if applicable)
  • [.] I added screenshots in the PR description (if change is visible to the user)
  • [.] I added a screenshot in the PR description showing a library with a single entry with me as author and as title the issue number
  • [.] I described the change in CHANGELOG.md in a way that can be understood by the average user (if change is visible to the user)
  • [.] I checked the user documentation for up to dateness and submitted a pull request to our user documentation repository

@qodo-free-for-open-source-projects
Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Fix hash warning in BibTeX string formatting

🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Fix incorrect hash warning triggered during entry editing
• Correct control flow logic in string formatting method
• Move warning condition inside proper conditional block
Diagram
flowchart LR
  A["String formatting logic"] -- "Fix control flow" --> B["Correct conditional nesting"]
  B -- "Move warning check" --> C["Warning only on unmatched hash"]
  C -- "Result" --> D["No spurious warnings"]
Loading

Grey Divider

File Changes

1. jablib/src/main/java/org/jabref/logic/bibtex/FieldWriter.java 🐞 Bug fix +9/-9

Fix hash warning control flow in string formatting

• Fix control flow by moving the pos2 == -1 check inside the else block
• Correct indentation to reflect proper nesting of conditional logic
• Prevent spurious hash warnings when processing BibTeX strings
• Warning now only triggers when hash character is genuinely unmatched

jablib/src/main/java/org/jabref/logic/bibtex/FieldWriter.java


Grey Divider

Qodo Logo

@qodo-free-for-open-source-projects
Copy link
Copy Markdown
Contributor

qodo-free-for-open-source-projects Bot commented Apr 20, 2026

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Remediation recommended

1. Warning suppression untested 🐞 Bug ⚙ Maintainability
Description
formatAndResolveStrings now only logs the “character # is not allowed…” warning when a starting
# exists but no closing # is found, but no unit test would fail if this behavior regresses and
starts warning for hash-free values again. This makes it easy to reintroduce the original log-spam
problem without detection.
Code

jablib/src/main/java/org/jabref/logic/bibtex/FieldWriter.java[R195-203]

+                if (pos2 == -1) {
+                    pos1 = content.length();
+                    LOGGER.warn("The character {} is not allowed in BibTeX strings unless escaped as in '\\\\{}'. "
+                                    + "In JabRef, use pairs of # characters to indicate a string. "
+                                    + "Field value: {}",
+                            BIBTEX_STRING_START_END_SYMBOL,
+                            BIBTEX_STRING_START_END_SYMBOL,
+                            content);
+                }
Evidence
The warning is emitted inside the pos2 == -1 branch in formatAndResolveStrings, which is the
behavior adjusted by this PR. Existing unit tests cover hash-enclosed strings and some edge cases
(e.g., #jan# - #feb#, #text, \#text, te##xt), but they do not verify the warning behavior
(or its absence) for a hash-free month value that originally caused the false warning spam.

jablib/src/main/java/org/jabref/logic/bibtex/FieldWriter.java[175-223]
jablib/src/test/java/org/jabref/logic/bibtex/FieldWriterTest.java[157-179]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The PR fixes when `FieldWriter.formatAndResolveStrings` logs a warning about `#`, but there is no unit test that would catch a regression (e.g., warning again when the input contains no `#`).

### Issue Context
This PR’s user-visible effect is “no warning about hashes” during editing; since this is primarily log/diagnostic behavior, it’s easy to break in the future without a targeted test.

### Fix Focus Areas
- jablib/src/test/java/org/jabref/logic/bibtex/FieldWriterTest.java[157-179]
- jablib/src/main/java/org/jabref/logic/bibtex/FieldWriter.java[175-205]

### Suggested test approach
1. Add a test like `monthWithoutHashDoesNotWarn()` that calls `writer.write(StandardField.MONTH, "January")` and asserts the output is `{January}`.
2. Capture and assert logging behavior for `FieldWriter` so the test also verifies that *no* WARN is emitted for hash-free input, while an unpaired-hash input (e.g., `"#text"`) *does* emit the WARN.
  - If the project already has a preferred SLF4J/logback test appender utility, use that; otherwise add a minimal test appender for this test class.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@calixtus calixtus added this pull request to the merge queue Apr 20, 2026
@github-actions github-actions Bot added the status: to-be-merged PRs which are accepted and should go into the merge-queue. label Apr 20, 2026
Merged via the queue into main with commit 77ade24 Apr 20, 2026
81 of 89 checks passed
@calixtus calixtus deleted the fix-hash-warning branch April 20, 2026 19:59
Siedlerchr added a commit that referenced this pull request Apr 22, 2026
* upstream/main:
  Remove review-to-comment migration (#15609)
  Chore(deps): Bump com.uber.nullaway:nullaway in /versions (#15615)
  Chore(deps): Bump javafx from 26 to 26.0.1 in /versions (#15614)
  Update dependency org.openjfx:javafx-base to v26.0.1 (#15608)
  Chore(deps): Bump org.openrewrite.rewrite from 7.30.0 to 7.31.0 (#15602)
  OpenOffice Refactor subtasks (#15380)
  Chore(deps): Bump org.hisp.dhis:json-tree in /versions (#15606)
  Chore(deps): Bump com.autonomousapps:dependency-analysis-gradle-plugin (#15604)
  Chore(deps): Bump com.autonomousapps:dependency-analysis-gradle-plugin (#15605)
  Chore(deps): Bump org.openrewrite.recipe:rewrite-recipe-bom (#15603)
  Fix hash warning (#15596)
  Update testlens (#15595)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Wrong warnings in log

3 participants