Add valiation to cite command by koppor · Pull Request #15063 · JabRef/jabref

koppor · 2026-02-09T00:57:57Z

Closes JabRef#723

Closes JabRef#725 (because ported to here)

Steps to test

Open preferences
Go to "External programs"
Modify citation key

JabRef#723

Mandatory checks

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 is understandable for the average user (if change is visible to the user)
[/] I checked the user 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 updating file(s) in https://github.com/JabRef/user-documentation/tree/main/en.

Fix WARN for incorrect completeCiteCommand Co-authored-by: koppor <1366654+koppor@users.noreply.github.com>

…290637

qodo-free-for-open-source-projects · 2026-02-09T00:58:17Z

Review Summary by Qodo

Add validation warning for citation command placeholders

🐞 Bug fix ✨ Enhancement

Walkthroughs

Description

• Add validation for citation command placeholders in preferences
• Display warning when cite command missing required key1/key2 placeholders
• Make CITE_KEY constants public for validator access
• Change log level from info to warn for invalid cite commands

Diagram

flowchart LR
  A["User modifies cite command"] --> B["ExternalTab initializes validation"]
  B --> C["citeCommandValidator checks placeholders"]
  C --> D{Contains key1 and key2?}
  D -->|Yes| E["No warning displayed"]
  D -->|No| F["Warning message shown to user"]
  G["CitationCommandString.from()"] --> H["Logs warning if invalid"]

File Changes

1. jabgui/src/main/java/org/jabref/gui/preferences/external/ExternalTab.java ✨ Enhancement +1/-0

Add cite command validation visualization
• Initialize validation visualization for cite command field
• Bind cite command validation status to UI decorator
jabgui/src/main/java/org/jabref/gui/preferences/external/ExternalTab.java

2. jabgui/src/main/java/org/jabref/gui/preferences/external/ExternalTabViewModel.java ✨ Enhancement +16/-0

Implement cite command validation logic
• Add citeCommandValidator field with validation logic
• Validate that cite command contains both key1 and key2 placeholders
• Add citeCommandValidationStatus() getter method
• Include citeCommandValidator in validateSettings() checks
jabgui/src/main/java/org/jabref/gui/preferences/external/ExternalTabViewModel.java

3. jablib/src/main/java/org/jabref/logic/push/CitationCommandString.java 🐞 Bug fix +3/-3

Expose citation key constants and improve logging
• Make CITE_KEY1 and CITE_KEY2 constants public for external access
• Change log level from info to warn for invalid cite commands
• Reorder constants to appear before logger declaration
jablib/src/main/java/org/jabref/logic/push/CitationCommandString.java

View more (3)

4. jablib/src/test/java/org/jabref/preferences/CitationCommandStringTest.java 🧪 Tests +8/-0

Expand citation command string test coverage
• Add test cases for edge cases with empty prefix/suffix
• Add test case for reversed key order (key2 before key1)
jablib/src/test/java/org/jabref/preferences/CitationCommandStringTest.java

5. CHANGELOG.md 📝 Documentation +1/-0

Update changelog with validation warning
• Document new warning feature for citation command validation
CHANGELOG.md

6. jablib/src/main/resources/l10n/JabRef_en.properties 📝 Documentation +1/-0

Add localization for validation message
• Add localization string for cite command validation warning message
jablib/src/main/resources/l10n/JabRef_en.properties

jabref-machine · 2026-02-09T01:00:35Z

The title of the pull request must not start with "Fix for issue xyz". Please use a concise one-line summary that explains what the fix or change actually does. Example of a good title: "Prevent crash when importing malformed BibTeX entries".

qodo-free-for-open-source-projects · 2026-02-09T01:03:57Z

Code Review by Qodo

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

✅ 1. Changelog says place holders 📘 Rule violation ✓ Correctness

Description

• The new changelog entry uses the misspelling place holders instead of placeholders.
• This violates the requirement to keep changelog/user-facing text free of typographical errors and
  reduces professionalism/clarity.

Code

CHANGELOG.md[25]

+- We added a warning if the citation command does not contain the expected place holders.

Evidence
PR Compliance ID 29 requires typo-free changelog text. The added changelog line contains the
misspelling place holders.
CHANGELOG.md[25-25]
Best Practice: Learned patterns

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 changelog entry contains a typographical error: `place holders` should be `placeholders`.
## Issue Context
Compliance requires changelog text to be free of misspellings.
## Fix Focus Areas
- CHANGELOG.md[25-25]

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

2. Warning blocks preferences 🐞 Bug ✓ Correctness

Description

• The new cite-command validator emits a ValidationMessage.warning, but it is added to the
  CompositeValidator used by validateSettings(), which blocks saving/closing when ValidationStatus is
  not valid.
• In this codebase, warnings still flip ValidationStatus.validProperty()/isValid to false (used
  elsewhere for a “duplicate DOI” warning), so users with a non-conforming cite command will be
  prevented from saving preferences and will see an error dialog for a warning-level message.

Code

jabgui/src/main/java/org/jabref/gui/preferences/external/ExternalTabViewModel.java[R166-170]

+        validator.addValidators(citeCommandValidator);
+
       ValidationStatus validationStatus = validator.getValidationStatus();
       if (!validationStatus.isValid()) {
           validationStatus.getHighestMessage().ifPresent(message ->

Evidence
ExternalTabViewModel adds citeCommandValidator to the validator used by validateSettings and blocks
when !validationStatus.isValid(), showing the highest message in an error dialog. The
citeCommandValidator itself is configured as a warning. Elsewhere in the GUI, a validator producing
ValidationMessage.warning is treated as invalid via validProperty(), demonstrating that warnings are
not merely informational in this app’s validation wiring.
jabgui/src/main/java/org/jabref/gui/preferences/external/ExternalTabViewModel.java[93-101]
jabgui/src/main/java/org/jabref/gui/preferences/external/ExternalTabViewModel.java[154-173]
jabgui/src/main/java/org/jabref/gui/newentry/NewEntryViewModel.java[168-176]
jabgui/src/main/java/org/jabref/gui/newentry/NewEntryView.java[302-309]

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 cite command validator is configured as a *warning*, but it is added to `validateSettings()`’s blocking `CompositeValidator`. In this app’s validation usage, warnings still cause `ValidationStatus` to be invalid, so this will prevent users from closing/saving Preferences and will surface the warning via an error dialog.
### Issue Context
This PR aims to “add a warning if the citation command does not contain the expected place holders”, which suggests non-blocking guidance. However, adding it to `validateSettings()` makes it blocking.
### Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/preferences/external/ExternalTabViewModel.java[93-101]
- jabgui/src/main/java/org/jabref/gui/preferences/external/ExternalTabViewModel.java[154-174]
### Implementation notes
Choose one:
1) **Non-blocking warning**: remove `citeCommandValidator` from the `CompositeValidator` in `validateSettings()` and rely on the visual decoration; optionally show a non-blocking warning dialog.
2) **Blocking validation**: change `ValidationMessage.warning(...)` to `ValidationMessage.error(...)` and adjust message/changelog to match enforced behavior.

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

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

qodo-free-for-open-source-projects · 2026-02-09T01:03:58Z

 - We added the ability to copy selected text from AI chat interface. [#14655](https://github.com/JabRef/jabref/issues/14655)
 - We added cover images for books, which will display in entry previews if available, and can be automatically downloaded when adding an entry via ISBN. [#10120](https://github.com/JabRef/jabref/issues/10120)
 - We added API key configuration support for [OpenAlex](https://openalex.org/). [#15023](https://github.com/JabRef/jabref/pull/15023)
+- We added a warning if the citation command does not contain the expected place holders.


1. Changelog says place holders 📘 Rule violation ✓ Correctness

• The new changelog entry uses the misspelling place holders instead of placeholders. • This violates the requirement to keep changelog/user-facing text free of typographical errors and reduces professionalism/clarity.

Agent Prompt

## Issue description The changelog entry contains a typographical error: `place holders` should be `placeholders`. ## Issue Context Compliance requires changelog text to be free of misspellings. ## Fix Focus Areas - CHANGELOG.md[25-25]

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

qodo-free-for-open-source-projects · 2026-02-09T01:03:58Z

+        validator.addValidators(citeCommandValidator);
+
        ValidationStatus validationStatus = validator.getValidationStatus();
        if (!validationStatus.isValid()) {
            validationStatus.getHighestMessage().ifPresent(message ->


2. Warning blocks preferences 🐞 Bug ✓ Correctness

• The new cite-command validator emits a ValidationMessage.warning, but it is added to the CompositeValidator used by validateSettings(), which blocks saving/closing when ValidationStatus is not valid. • In this codebase, warnings still flip ValidationStatus.validProperty()/isValid to false (used elsewhere for a “duplicate DOI” warning), so users with a non-conforming cite command will be prevented from saving preferences and will see an error dialog for a warning-level message.

Agent Prompt

### Issue description The cite command validator is configured as a *warning*, but it is added to `validateSettings()`’s blocking `CompositeValidator`. In this app’s validation usage, warnings still cause `ValidationStatus` to be invalid, so this will prevent users from closing/saving Preferences and will surface the warning via an error dialog. ### Issue Context This PR aims to “add a warning if the citation command does not contain the expected place holders”, which suggests non-blocking guidance. However, adding it to `validateSettings()` makes it blocking. ### Fix Focus Areas - jabgui/src/main/java/org/jabref/gui/preferences/external/ExternalTabViewModel.java[93-101] - jabgui/src/main/java/org/jabref/gui/preferences/external/ExternalTabViewModel.java[154-174] ### Implementation notes Choose one: 1) **Non-blocking warning**: remove `citeCommandValidator` from the `CompositeValidator` in `validateSettings()` and rely on the visual decoration; optionally show a non-blocking warning dialog. 2) **Blocking validation**: change `ValidationMessage.warning(...)` to `ValidationMessage.error(...)` and adjust message/changelog to match enforced behavior.

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

@Siedlerchr This is OK, isn't it?

Ask❓

@Siedlerchr This is OK, isn't it?

Answer:

Not quite. With the current code, this is not “just a warning” in terms of behavior.

Because you add citeCommandValidator to the CompositeValidator in validateSettings():

validator.addValidators(citeCommandValidator);

and validateSettings() blocks closing/saving when validationStatus.isValid() is false, any warning-level ValidationMessage will still typically make ValidationStatus invalid in this codebase (as the bot noted). Result:

Users with a non-conforming cite command will be prevented from saving/closing Preferences

The warning text will be shown via showErrorDialogAndWait(...), i.e., as an error dialog

This contradicts the PR intention (“added a warning…”), which implies non-blocking guidance

Recommended fix (pick one based on intended UX):

Non-blocking warning (likely intended):

Keep the visual validation decoration (initVisualization(...)) so users see the warning inline

Remove citeCommandValidator from the blocking validateSettings() composite validator

Optionally show a non-blocking notification/toast instead of an error dialog if you still want extra visibility

Make it truly blocking validation:

Change ValidationMessage.warning(...) to ValidationMessage.error(...)

Update wording/Changelog to reflect that the cite command is now enforced (not merely warned)

Given the changelog text and issue description, option (1) seems the correct behavior.

Siedlerchr · 2026-02-09T08:48:36Z

+
        ValidationStatus validationStatus = validator.getValidationStatus();
        if (!validationStatus.isValid()) {
            validationStatus.getHighestMessage().ifPresent(message ->


The bot is right here I think

Probably yes, but requires some more refactoring / rewording, just to have to code more expressive. So i would just let it slip and create a follow-up issue.

junie-agent and others added 4 commits February 4, 2026 23:34

[issue-723]

1a94942

Fix WARN for incorrect completeCiteCommand Co-authored-by: koppor <1366654+koppor@users.noreply.github.com>

Merge remote-tracking branch 'origin/main' into junie/issue-723-21692…

91bb3ed

…290637

Initial line for CHANGELOG.md

1dfc18e

Add link to PR

998f8f5

koppor changed the title ~~Junie/issue 723 21692290637~~ Add valiation to cite command Feb 9, 2026

github-actions Bot added the status: changes-required Pull requests that are not yet complete label Feb 9, 2026

qodo-free-for-open-source-projects Bot reviewed Feb 9, 2026

View reviewed changes

Fix space in CHANGELOG

35a155d

Siedlerchr reviewed Feb 9, 2026

View reviewed changes

Siedlerchr enabled auto-merge February 9, 2026 19:38

Siedlerchr approved these changes Feb 9, 2026

View reviewed changes

Siedlerchr added this pull request to the merge queue Feb 9, 2026

Merged via the queue into JabRef:main with commit 8218465 Feb 9, 2026
56 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add valiation to cite command#15063

Add valiation to cite command#15063
Siedlerchr merged 5 commits into
mainfrom
junie/issue-723-21692290637

koppor commented Feb 9, 2026 •

edited

Loading

Uh oh!

qodo-free-for-open-source-projects Bot commented Feb 9, 2026

Uh oh!

jabref-machine commented Feb 9, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Feb 9, 2026 •

edited

Loading

Uh oh!

qodo-free-for-open-source-projects Bot Feb 9, 2026

Uh oh!

qodo-free-for-open-source-projects Bot Feb 9, 2026

Uh oh!

koppor Feb 9, 2026

Uh oh!

qodo-free-for-open-source-projects Bot Feb 9, 2026

Uh oh!

Siedlerchr Feb 9, 2026

Uh oh!

calixtus Feb 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

koppor commented Feb 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Steps to test

Mandatory checks

Uh oh!

qodo-free-for-open-source-projects Bot commented Feb 9, 2026

Review Summary by Qodo

Walkthroughs

File Changes

Uh oh!

jabref-machine commented Feb 9, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Feb 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

qodo-free-for-open-source-projects Bot Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

qodo-free-for-open-source-projects Bot Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

koppor Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

qodo-free-for-open-source-projects Bot Feb 9, 2026

Choose a reason for hiding this comment

Ask❓

Answer:

Uh oh!

Siedlerchr Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

calixtus Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

koppor commented Feb 9, 2026 •

edited

Loading

qodo-free-for-open-source-projects Bot commented Feb 9, 2026 •

edited

Loading