Skip to content

CSL4LibreOffice - J [Respect database preference, miscellaneous refactoring] - #16484

Merged
subhramit merged 32 commits into
JabRef:mainfrom
subhramit:lo-entry-lookup
Aug 6, 2026
Merged

CSL4LibreOffice - J [Respect database preference, miscellaneous refactoring]#16484
subhramit merged 32 commits into
JabRef:mainfrom
subhramit:lo-entry-lookup

Conversation

@subhramit

@subhramit subhramit commented Aug 3, 2026

Copy link
Copy Markdown
Member

Summary

This is a priming step for #11829 and #16380.
JStyle and BST respect the preference of "Look up BibTeX entries in the active tab only". CSL did not.
This causes an issue found by @pluto-han wherein he had the same entry open in two different libraries - thus our citation key generator would not resolve their citation keys to be distinct (a and b), but since during generation of bibliography all databases were always considered, they resulted in two different bibliography entries (which were same in content) in the references list.

This PR

  • makes CSL respect the preference (if enabled)

  • removes its complement option from the UI " (just ticked/unticked on one was enough)
    Before:
    image
    After:
    image

  • refactors CSL style method signatures to simplify code (e.g. gets rid of the supplier introduced in CSL4LibreOffice - F [Real-time switching of CSL Styles] #12472), get rid of redundant parameters

    • this reduces some code/steps in some lookup operations (used during citation rewrite) and thus improves performance during all insertion (rendering)/ refresh (re-rendering) operations - including "Cite" and "Generate/Update Bibliography"
    • Architectural refactoring was also involved, which separates GUI (OpenOfficePanel)/StateManager concerns from the backend (OOBibBase).

Steps to test

  1. Connect to an LO doc
  2. Select any CSL style
  3. Turn on "Look up BibTeX entries in the active tab only" from the LO panel's Settings
  4. Have two libraries open, lets say library 1 and 2.
  5. Have an entry in library 2 which has the same citation key as library 1 but lets say, a different journal name
  6. Cite that entry from library 1 (or 2)
  7. Select library 1 tab
  8. Generate bibliography - note that the journal name should correspond to library 1's data and there should be only one bibliography entry in the references list.
  9. Select library 2 tab
  10. Refresh bibliography - note that the journal name should change to library 2's data and there should be only one entry again in the list
pr_J.mp4

Related issues and pull requests

Closes NA

AI usage

🧠 + [Zed + GPT 5 (manually driven)] + quite a bit of handwritten code

Checklist

  • I own the copyright of the code submitted and I license it under the MIT license
  • If AI tools were used, I disclosed them in the "AI usage" section and reviewed, understood, and take full ownership of all AI-generated code
  • 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

Signed-off-by: subhramit <subhramit.bb@live.in>
Signed-off-by: subhramit <subhramit.bb@live.in>
Signed-off-by: subhramit <subhramit.bb@live.in>
Signed-off-by: subhramit <subhramit.bb@live.in>
Signed-off-by: subhramit <subhramit.bb@live.in>
Signed-off-by: subhramit <subhramit.bb@live.in>
Signed-off-by: subhramit <subhramit.bb@live.in>
Signed-off-by: subhramit <subhramit.bb@live.in>
@qodo-free-for-open-source-projects

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

LibreOffice CSL: respect active-tab lookup preference; simplify scope handling

🐞 Bug fix ✨ Enhancement 🧪 Tests 📝 Documentation 🕐 40+ Minutes

Grey Divider

AI Description

• Make LibreOffice CSL cite/bibliography lookup honor the active-tab-only database preference.
• Remove redundant “all open libraries” UI toggle and corresponding localization string.
• Refactor CSL adapter APIs to pass explicit lookup scope and reduce redundant updates.
Diagram

graph TD
  panel["OpenOfficePanel (UI)"] --> prefs["OpenOfficePreferences"]
  oobib["OOBibBase"] --> state["StateManager"] --> scope[("Lookup BibDatabases")]
  oobib --> adapter["CSLCitationOOAdapter"] --> doc["LibreOffice Doc"]
  oobib --> bib["CSL bibliography rebuild"] --> adapter
  scope --> adapter
  scope --> bib
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Keep Supplier> and filter by preference
  • ➕ Minimal API churn across CSL classes
  • ➕ Keeps “live” database list behavior without passing parameters everywhere
  • ➖ Still hides lookup scope behind indirection, making behavior harder to reason about
  • ➖ Encourages more implicit global state access inside CSL code
2. Introduce a dedicated LookupScope service (active-only vs all-open)
  • ➕ Centralizes scope logic and makes it reusable across JStyle/BST/CSL
  • ➕ Could standardize scope selection for all LibreOffice operations
  • ➖ Adds new abstraction and wiring overhead for a fairly small behavior change
  • ➖ More files/DI changes than needed for this fix

Recommendation: Current approach (compute lookup databases once in OOBibBase and pass explicitly) is the best trade-off: it makes scope explicit for CSL, fixes the preference mismatch, and removes reliance on a supplier/hidden state. Keep an eye on consistency with non-CSL paths (BST/JStyle) to ensure all formats derive scope the same way.

Files changed (9) +109 / -121

Enhancement (1) +4 / -17
OpenOfficePanel.javaReplace two radio options with a single active-tab-only checkbox +4/-17

Replace two radio options with a single active-tab-only checkbox

• Removes the separate “all open libraries” setting from the settings popup. Uses a single checkbox for active-tab-only and inverts it to the underlying useAllDatabases preference.

jabgui/src/main/java/org/jabref/gui/openoffice/OpenOfficePanel.java

Bug fix (1) +36 / -19
OOBibBase.javaDerive lookup databases from preference and pass into CSL operations +36/-19

Derive lookup databases from preference and pass into CSL operations

• Creates a helper to select lookup databases based on the OpenOffice preference (all open vs active). Updates CSL citation insertion and bibliography rebuild to pass the selected database scope explicitly and to prepare citation insertion up-front for fewer updates.

jabgui/src/main/java/org/jabref/gui/openoffice/OOBibBase.java

Refactor (4) +66 / -83
CSLBibliographyMark.javaThread lookup database scope through CSL bibliography population +6/-6

Thread lookup database scope through CSL bibliography population

• Changes bibliography rebuild/population APIs to accept the lookup database list and forwards it to CSLCitationOOAdapter for consistent citation updates during bibliography generation.

jablib/src/main/java/org/jabref/logic/openoffice/CSLBibliographyMark.java

CSLCitationOOAdapter.javaRemove database supplier; make lookup scope explicit for updates/conversion +44/-60

Remove database supplier; make lookup scope explicit for updates/conversion

• Eliminates the Supplier-based database access and replaces it with explicit lookupDatabases parameters for reference-mark conversion and citation restyling. Introduces prepareCitationInsertion to link Zotero citations and update style/type once per insertion path.

jablib/src/main/java/org/jabref/logic/openoffice/oocsltext/CSLCitationOOAdapter.java

CSLReferenceMarkManager.javaConvert reference marks using lookup databases (no contexts) +13/-14

Convert reference marks using lookup databases (no contexts)

• Refactors reference mark conversion to work directly with a list of BibDatabase, simplifying entry lookup by citation key and aligning conversion behavior with the selected lookup scope.

jablib/src/main/java/org/jabref/logic/openoffice/oocsltext/CSLReferenceMarkManager.java

CSLUpdateBibliography.javaPropagate lookup database scope into CSL bibliography rebuild +3/-3

Propagate lookup database scope into CSL bibliography rebuild

• Updates rebuildCSLBibliography signature to accept lookupDatabases and passes it through to CSLBibliographyMark to keep scope consistent during rebuild.

jablib/src/main/java/org/jabref/logic/openoffice/oocsltext/CSLUpdateBibliography.java

Tests (1) +1 / -1
OOBibBaseTest.javaAdjust mocks to updated CSL bibliography rebuild signature +1/-1

Adjust mocks to updated CSL bibliography rebuild signature

• Updates verification of CSLUpdateBibliography.rebuildCSLBibliography calls to match the new parameter order/signature including lookup databases.

jabgui/src/test/java/org/jabref/gui/openoffice/OOBibBaseTest.java

Documentation (2) +2 / -1
CHANGELOG.mdDocument CSL lookup-scope behavior and UI simplification +2/-0

Document CSL lookup-scope behavior and UI simplification

• Adds an entry noting CSL now respects the active-tab-only lookup preference. Notes removal of the redundant “all open libraries” toggle from the LibreOffice panel.

CHANGELOG.md

JabRef_en.propertiesRemove redundant LibreOffice lookup option string +0/-1

Remove redundant LibreOffice lookup option string

• Deletes the localization key for “Look up BibTeX entries in all open libraries” after consolidating the UI to a single active-tab-only toggle.

jablib/src/main/resources/l10n/JabRef_en.properties

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Removed-setting entry lacks link ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
The new Unreleased CHANGELOG removal entry is missing a finalized issue/PR link, reducing
traceability and failing the required Unreleased entry format.
Code

CHANGELOG.md[133]

+- We removed the redundant "Look up BibTeX entries in all open libraries" setting from the LibreOffice panel, which is now the toggle of "Look up BibTeX entries in the active tab only".
Evidence
PR Compliance ID 52 requires that Unreleased CHANGELOG entries include finalized issue links
(preferred) or PR links (fallback) for traceability. The newly added removal line (including the
CSL citations and bibliography... entry) contains no such link, demonstrating non-compliance with
the required format.

CHANGELOG.md[133-133]
CHANGELOG.md[14-14]
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 new Unreleased CHANGELOG entry (describing removal/CSL lookup behavior) is missing a finalized issue/PR link, which breaks the required Unreleased entry format and reduces traceability.
## Issue Context
PR Compliance ID 52 requires CHANGELOG items under **Unreleased** to include a valid finalized issue link (preferred) or PR link (fallback) for traceability. The newly added removal line currently has no link.
## Fix Focus Areas
- CHANGELOG.md[133-133]
- CHANGELOG.md[14-14]

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


2. Empty CSL citation crash ✓ Resolved 🐞 Bug ☼ Reliability
Description
CSLCitationOOAdapter.updateAllCitationsWithNewStyle can build an empty per-mark entry list when
cited keys are outside the selected lookup databases (e.g., active-tab-only). It then calls CSL
citation generation with an empty list, which can throw at runtime (via getFirst()), breaking
citation insertion or bibliography refresh.
Code

jablib/src/main/java/org/jabref/logic/openoffice/oocsltext/CSLCitationOOAdapter.java[R465-468]

+        List<BibEntry> citedEntries = lookupDatabases.stream()
+                                                     .flatMap(db -> db.getEntries().stream())
+                                                     .filter(this::isCitedEntry)
+                                                     .toList();
Evidence
After this PR, citation regeneration is driven by lookupDatabases and per-mark entry resolution
can legitimately return an empty list when keys are outside the configured scope. The
NORMAL-citation regeneration path then calls CitationStyleGenerator.generateCitation with that
empty list; the generator delegates to CSLAdapter.makeCitation, which unconditionally calls
getFirst() on the citeproc result and does not guard against an empty input/result, so this path
can throw at runtime during cite/bibliography operations.

jablib/src/main/java/org/jabref/logic/openoffice/oocsltext/CSLCitationOOAdapter.java[457-518]
jablib/src/main/java/org/jabref/logic/citationstyle/CitationStyleGenerator.java[32-42]
jablib/src/main/java/org/jabref/logic/citationstyle/CSLAdapter.java[57-68]
jabgui/src/main/java/org/jabref/gui/openoffice/OOBibBase.java[117-126]

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

## Issue description
When CSL lookup is restricted (e.g., active-tab-only), some existing citation marks in the document may reference citation keys not present in the provided `lookupDatabases`. In `updateAllCitationsWithNewStyle`, those marks resolve to an empty `entries` list; for NORMAL citations this is passed into `CitationStyleGenerator.generateCitation`, which calls into `CSLAdapter.makeCitation(...).getFirst()` and can throw when no citation is produced.
### Issue Context
This PR intentionally limits CSL lookups to the configured database scope. The citation-refresh code must therefore tolerate “unresolvable in current scope” marks without crashing.
### Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/openoffice/oocsltext/CSLCitationOOAdapter.java[457-505]
- jablib/src/main/java/org/jabref/logic/citationstyle/CSLAdapter.java[57-68]
- jablib/src/main/java/org/jabref/logic/citationstyle/CitationStyleGenerator.java[32-42]
### Suggested fix
- In `updateAllCitationsWithNewStyle`, before regenerating a mark, detect unresolved keys (e.g., `entries.isEmpty()` or `entries.size() != citationKeys.size()`). For unresolved marks, **do not** call `createCitationText`/`generateCitation`; instead:
- skip updating that mark (leave existing text/mark name as-is), and optionally log at debug/warn with the missing keys, OR
- write a safe placeholder text (if desired), but avoid throwing.
- Optionally add a defensive guard in `CSLAdapter.makeCitation` (or `CitationStyleGenerator.generateCitation`) to return a safe fallback when `bibEntries.isEmpty()` rather than calling `getFirst()`.

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


To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

Comment thread CHANGELOG.md Outdated
Comment thread jabgui/src/main/java/org/jabref/gui/openoffice/OOBibBase.java Outdated
@github-actions github-actions Bot added status: changes-required Pull requests that are not yet complete and removed status: no-bot-comments labels Aug 3, 2026
Signed-off-by: subhramit <subhramit.bb@live.in>
Signed-off-by: subhramit <subhramit.bb@live.in>
@subhramit
subhramit requested a review from Siedlerchr August 3, 2026 22:53
@github-actions github-actions Bot added status: no-bot-comments and removed status: changes-required Pull requests that are not yet complete labels Aug 3, 2026
Signed-off-by: subhramit <subhramit.bb@live.in>
Signed-off-by: subhramit <subhramit.bb@live.in>
Signed-off-by: subhramit <subhramit.bb@live.in>
Signed-off-by: subhramit <subhramit.bb@live.in>
@github-actions github-actions Bot added status: no-bot-comments and removed status: changes-required Pull requests that are not yet complete labels Aug 4, 2026
Signed-off-by: subhramit <subhramit.bb@live.in>
Signed-off-by: subhramit <subhramit.bb@live.in>
Signed-off-by: subhramit <subhramit.bb@live.in>
This reverts commit 229be6e.
@subhramit subhramit added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Aug 4, 2026
@github-actions github-actions Bot added status: changes-required Pull requests that are not yet complete status: no-bot-comments and removed status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers status: no-bot-comments status: changes-required Pull requests that are not yet complete labels Aug 4, 2026
@pluto-han

Copy link
Copy Markdown
Collaborator

LGTM, just tiny nitpicks

Comment thread jabgui/src/main/java/org/jabref/gui/openoffice/OOBibBase.java Outdated
Signed-off-by: subhramit <subhramit.bb@live.in>

@pluto-han pluto-han left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🏄

@subhramit
subhramit added this pull request to the merge queue Aug 6, 2026
@subhramit
subhramit removed this pull request from the merge queue due to a manual request Aug 6, 2026
@subhramit
subhramit added this pull request to the merge queue Aug 6, 2026
@subhramit
subhramit removed this pull request from the merge queue due to a manual request Aug 6, 2026
@subhramit
subhramit merged commit 30ae9bf into JabRef:main Aug 6, 2026
62 checks passed
@subhramit
subhramit deleted the lo-entry-lookup branch August 6, 2026 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants