Skip to content

Embed in-text nature in reference marks for CSL citations#15381

Merged
subhramit merged 3 commits into
JabRef:mainfrom
pluto-han:fix-for-issue-15370
Mar 27, 2026
Merged

Embed in-text nature in reference marks for CSL citations#15381
subhramit merged 3 commits into
JabRef:mainfrom
pluto-han:fix-for-issue-15370

Conversation

@pluto-han

@pluto-han pluto-han commented Mar 20, 2026

Copy link
Copy Markdown
Collaborator

Related issues and pull requests

Closes #15370

PR Description

As a follow-up PR of #15369, this PR embeded in-text nature in the reference mark. Now, in-text property can be retained when JabRef is restarted.

Steps to test

Before:
Cite with in-text IEEE
image
Restart JabRef and sync bibliography -> Citations refreshed to non-in-text
image

Now:
Remain in-text citations
image

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

Add in-text citation marker support to reference marks

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Add in-text citation marker support to reference marks
• Track in-text citation status throughout document lifecycle
• Update reference mark format to include optional IN_TEXT marker
• Propagate in-text flag through citation insertion and style updates
Diagram
flowchart LR
  A["ReferenceMark<br/>parsing"] -->|"parse IN_TEXT<br/>marker"| B["inText flag<br/>stored"]
  B -->|"pass to<br/>CSLReferenceMark"| C["CSLReferenceMark<br/>creation"]
  C -->|"build reference<br/>name with marker"| D["Reference mark<br/>in document"]
  E["insertCitation<br/>insertInTextCitation"] -->|"pass inText<br/>parameter"| F["insertReferences<br/>method"]
  F -->|"create mark with<br/>inText flag"| G["CSLReferenceMarkManager<br/>tracks in-text usage"]
  G -->|"update style with<br/>inText awareness"| H["updateMarkAndTextWithNewStyle<br/>preserves marker"]
Loading

Grey Divider

File Changes

1. jablib/src/main/java/org/jabref/logic/openoffice/ReferenceMark.java ✨ Enhancement +13/-3

Add in-text citation marker parsing and tracking

• Add IN_TEXT_MARKER constant for marking in-text citations
• Update regex pattern to optionally capture IN_TEXT marker at end of reference mark name
• Add inText boolean field to track in-text citation status
• Update constructor to accept inText parameter
• Parse and set inText flag in parse() method
• Add isInText() getter method
• Update debug logging to include in-text status

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


2. jablib/src/main/java/org/jabref/logic/openoffice/oocsltext/CSLCitationOOAdapter.java ✨ Enhancement +11/-7

Thread in-text citation flag through citation operations

• Add inTextUsed field to track if document contains in-text citations
• Initialize inTextUsed from mark manager after reading existing marks
• Pass isInText parameter to insertReferences() method calls
• Update insertReferences() signature to accept isInText boolean
• Pass in-text flag to mark manager's insertReferenceIntoOO() method
• Update updateAllCitationsWithNewStyle() to pass in-text style flag to mark manager
• Refresh inTextUsed status after bibliography operations

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


3. jablib/src/main/java/org/jabref/logic/openoffice/oocsltext/CSLReferenceMark.java ✨ Enhancement +10/-5

Store and propagate in-text citation status

• Add isInText field to track in-text citation status
• Update constructor to extract and store in-text status from ReferenceMark
• Update of() factory method to accept inText parameter
• Update buildReferenceName() to append IN_TEXT_MARKER when needed
• Pass inText flag when creating ReferenceMark instances
• Preserve in-text status in updateName() method

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


View more (2)
4. jablib/src/main/java/org/jabref/logic/openoffice/oocsltext/CSLReferenceMarkManager.java ✨ Enhancement +32/-9

Manage in-text citation tracking and marker updates

• Add isInTextCite field to track if any marks are in-text citations
• Update createReferenceMark() to accept inText parameter
• Update insertReferenceIntoOO() to accept and pass inText flag
• Track in-text status when creating and reading marks
• Update getUpdatedReferenceMarkNameWithNewNumbers() to preserve IN_TEXT_MARKER
• Update updateMarkAndTextWithNewStyle() to handle in-text marker addition/removal
• Add hasInTextMarks() method to check if document has in-text citations
• Reset isInTextCite flag when reading existing marks

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


5. jablib/src/test/java/org/jabref/logic/openoffice/ReferenceMarkTest.java 🧪 Tests +17/-12

Add in-text citation marker parsing tests

• Add expectedInText parameter to validParsing() test method
• Update all test cases to include in-text expectation (false for existing cases)
• Add new test case for in-text citation with IN_TEXT marker
• Assert isInText() value in test validation

jablib/src/test/java/org/jabref/logic/openoffice/ReferenceMarkTest.java


Grey Divider

Qodo Logo

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

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

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (2) 📎 Requirement gaps (0) 📐 Spec deviations (0)

Grey Divider


Action required

1. parts[1] accessed unsafely 📘 Rule violation ⛯ Reliability
Description
getUpdatedReferenceMarkNameWithNewNumbers accesses parts[1] after oldName.split(" ") without
first validating parts.length >= 2, which can throw ArrayIndexOutOfBoundsException on
malformed/short names. This violates the guideline to guard against missing/empty values and unsafe
access patterns.
Code

jablib/src/main/java/org/jabref/logic/openoffice/oocsltext/CSLReferenceMarkManager.java[R177-183]

 private String getUpdatedReferenceMarkNameWithNewNumbers(String oldName, List<Integer> newNumbers) {
     String[] parts = oldName.split(" ");
-        if (parts[0].startsWith(ReferenceMark.PREFIXES[0]) && parts[1].startsWith(ReferenceMark.PREFIXES[1]) && parts.length >= 3) {
+        boolean inText = ReferenceMark.IN_TEXT_MARKER.equals(parts[parts.length - 1]);
+        int uniqueIdIndex = inText ? parts.length - 2 : parts.length - 1;
+
+        if (parts[0].startsWith(ReferenceMark.PREFIXES[0]) && parts[1].startsWith(ReferenceMark.PREFIXES[1]) && uniqueIdIndex >= 2) {
         StringBuilder newName = new StringBuilder();
-            for (int i = 0; i < parts.length - 1; i += 2) {
Evidence
PR Compliance ID 34 requires guarding against missing/empty values and avoiding unsafe access. The
updated code computes parts and then evaluates parts[1].startsWith(...) in the if condition
without checking the array length first, which can crash at runtime.

jablib/src/main/java/org/jabref/logic/openoffice/oocsltext/CSLReferenceMarkManager.java[177-183]
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
`parts[1]` is accessed without verifying `parts.length &amp;gt;= 2`, risking `ArrayIndexOutOfBoundsException` for malformed reference mark names.
## Issue Context
This happens in `getUpdatedReferenceMarkNameWithNewNumbers`, where `oldName.split(&amp;quot; &amp;quot;)` may produce fewer than two tokens.
## Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/openoffice/oocsltext/CSLReferenceMarkManager.java[177-198]

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


2. INTEXT marker mismatch📘 Rule violation ✓ Correctness
Description
A new test case uses the reference mark suffix INTEXT, but production parsing/building uses
IN_TEXT, so the test will not match the implemented format and will fail (or validate the wrong
behavior). This violates the requirement to keep tests high quality and aligned with actual behavior
changes.
Code

jablib/src/test/java/org/jabref/logic/openoffice/ReferenceMarkTest.java[R82-85]

+                Arguments.of(
+                        "JABREF_key8 CID_7 uniqueId8 INTEXT",
+                        List.of("key8"), List.of(7), "uniqueId8", true
             )
Evidence
PR Compliance ID 22 requires behavior changes to be covered with correct, deterministic tests. The
added test expects INTEXT to parse as in-text, but ReferenceMark.IN_TEXT_MARKER is IN_TEXT and
the parsing regex only accepts that exact marker, so the test case is inconsistent with the
implementation.

AGENTS.md
jablib/src/test/java/org/jabref/logic/openoffice/ReferenceMarkTest.java[82-85]
jablib/src/main/java/org/jabref/logic/openoffice/ReferenceMark.java[15-21]

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 newly added test case uses `INTEXT`, but the implementation uses `IN_TEXT` (`ReferenceMark.IN_TEXT_MARKER`) and only matches that in the regex.
## Issue Context
Either the test should use `IN_TEXT`, or the production code should explicitly support parsing/building the legacy `INTEXT` marker (if backward compatibility is required).
## Fix Focus Areas
- jablib/src/test/java/org/jabref/logic/openoffice/ReferenceMarkTest.java[82-85]
- jablib/src/main/java/org/jabref/logic/openoffice/ReferenceMark.java[15-66]

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



Remediation recommended

3. createReferenceMark boolean parameter 📘 Rule violation ⚙ Maintainability
Description
New/changed public methods introduce boolean flag parameters (e.g., inText), which makes call
sites harder to read and maintain. This conflicts with the guideline to avoid boolean parameters in
public APIs in favor of intent-revealing methods.
Code

jablib/src/main/java/org/jabref/logic/openoffice/oocsltext/CSLReferenceMarkManager.java[R62-65]

+    public CSLReferenceMark createReferenceMark(List<BibEntry> entries, boolean inText) throws Exception {
     List<String> citationKeys = entries.stream()
                                        .map(entry -> entry.getCitationKey().orElse(CUID.randomCUID2(8).toString()))
                                        .collect(Collectors.toList());
Evidence
PR Compliance ID 15 prohibits introducing boolean parameters in public method signatures. The PR
changes add boolean inText to multiple public APIs, including
CSLReferenceMarkManager.createReferenceMark(...),
CSLReferenceMarkManager.insertReferenceIntoOO(...), and CSLReferenceMark.of(...), and adds a
boolean parameter to a public ReferenceMark constructor.

AGENTS.md
jablib/src/main/java/org/jabref/logic/openoffice/oocsltext/CSLReferenceMarkManager.java[62-80]
jablib/src/main/java/org/jabref/logic/openoffice/oocsltext/CSLReferenceMark.java[30-36]
jablib/src/main/java/org/jabref/logic/openoffice/ReferenceMark.java[44-50]

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

## Issue description
Public APIs were changed to include boolean flag parameters (e.g., `inText`), which reduces readability and violates the project guideline.
## Issue Context
New/changed signatures include `createReferenceMark(..., boolean inText)`, `insertReferenceIntoOO(..., boolean inText)`, and `CSLReferenceMark.of(..., boolean inText, ...)`.
## Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/openoffice/oocsltext/CSLReferenceMarkManager.java[62-81]
- jablib/src/main/java/org/jabref/logic/openoffice/oocsltext/CSLReferenceMark.java[30-37]
- jablib/src/main/java/org/jabref/logic/openoffice/ReferenceMark.java[44-50]

ⓘ 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

Comment on lines 177 to 183
private String getUpdatedReferenceMarkNameWithNewNumbers(String oldName, List<Integer> newNumbers) {
String[] parts = oldName.split(" ");
if (parts[0].startsWith(ReferenceMark.PREFIXES[0]) && parts[1].startsWith(ReferenceMark.PREFIXES[1]) && parts.length >= 3) {
boolean inText = ReferenceMark.IN_TEXT_MARKER.equals(parts[parts.length - 1]);
int uniqueIdIndex = inText ? parts.length - 2 : parts.length - 1;

if (parts[0].startsWith(ReferenceMark.PREFIXES[0]) && parts[1].startsWith(ReferenceMark.PREFIXES[1]) && uniqueIdIndex >= 2) {
StringBuilder newName = new StringBuilder();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Action required

1. parts[1] accessed unsafely 📘 Rule violation ⛯ Reliability

getUpdatedReferenceMarkNameWithNewNumbers accesses parts[1] after oldName.split(" ") without
first validating parts.length >= 2, which can throw ArrayIndexOutOfBoundsException on
malformed/short names. This violates the guideline to guard against missing/empty values and unsafe
access patterns.
Agent Prompt
## Issue description
`parts[1]` is accessed without verifying `parts.length >= 2`, risking `ArrayIndexOutOfBoundsException` for malformed reference mark names.

## Issue Context
This happens in `getUpdatedReferenceMarkNameWithNewNumbers`, where `oldName.split(" ")` may produce fewer than two tokens.

## Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/openoffice/oocsltext/CSLReferenceMarkManager.java[177-198]

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

@testlens-app

This comment has been minimized.

@github-actions github-actions Bot added the status: changes-required Pull requests that are not yet complete label Mar 20, 2026
@testlens-app

This comment has been minimized.


OOText ooText = OOFormat.setLocaleNone(OOText.fromString(formattedCitation));
insertReferences(cursor, entries, ooText, isNumericStyle);
insertReferences(cursor, entries, ooText, isNumericStyle, false);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

boolean could be replaced with an enum for better readability.

@pluto-han pluto-han Mar 24, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@subhramit Just for more suggestions, should we replace true/false with enum?

@subhramit subhramit Mar 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Carl is referring to readability - such that when someone reads this line, they don't have to infer what these boolean values are conveying to the call.
There are a couple of ways. For simplicity, you can maybe define a class constant IN_TEXT = true and use !IN_TEXT here (and its inverse at other places).
Enum is also a good idea, although more beneficial if there are three or more possible values involved.

@calixtus calixtus Mar 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I meant introduce a simple enum CITATION_FORM { IN_TEXT, OUT } (rename however you like :-) ) and then you can use that instead of the boolean var. Its about replacing the boolean in the method signature with something that makes more sense. A constant wouldn't force meaningful code.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@subhramit subhramit Mar 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I meant introduce a simple enum CITATION_FORM { IN_TEXT, OUT } (rename however you like :-) ) and then you can use that instead of the boolean var. Its about replacing the boolean in the method signature with something that makes more sense. A constant wouldn't force meaningful code.

Yeah and eventually we'll support INVISIBLE too as well here so we'll need enum anyway - we can lay the foundation here itself.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yeah a enum makes more sense in this case, I was misunderstanding. Thank you! @subhramit @calixtus

@subhramit subhramit left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

On first look, the direction and implementation seems correct. Good work,
I have two suggestions (apart from the separate comments above) -

  1. How about we put the intext marker somewhere in the middle like JStyles, rather than after the UID? Can you evaluate if the parsing/intext marker addition logic will become less neat? (To me it looks that it should be similar).
  2. Can you extend the solution to invisible citations too, to support the full citation nature suite? (This can be done as a follow-up PR)

@subhramit subhramit changed the title Embed in-text nature in reference mark Embed in-text nature in reference marks for CSL citations Mar 21, 2026
@pluto-han

Copy link
Copy Markdown
Collaborator Author

How about we put the intext marker somewhere in the middle like JStyles, rather than after the UID? Can you evaluate if the parsing/intext marker addition logic will become less neat? (To me it looks that it should be similar).

The logic is similar but the code IMHO could be more complex. If intext marker is after UID, the code is rather clear, and it does not break the original CSL format. But if we put it in the middle, some if logic could be complex.

Can you extend the solution to invisible citations too, to support the full citation nature suite? (This can be done as a follow-up PR)

Yes. From my tests, "insert empty citation" has the same problem with #15369. I will fix it in a follow-up PR.

@testlens-app

testlens-app Bot commented Mar 24, 2026

Copy link
Copy Markdown

✅ All tests passed ✅

🏷️ Commit: 8df57fa
▶️ Tests: 10204 executed
⚪️ Checks: 79/79 completed


Learn more about TestLens at testlens.app.

@subhramit subhramit left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good work!

@subhramit subhramit requested a review from Siedlerchr March 26, 2026 17:49
@subhramit subhramit added the status: awaiting-second-review For non-trivial changes label Mar 26, 2026
@subhramit

subhramit commented Mar 26, 2026

Copy link
Copy Markdown
Member

@pluto-han just missed that refactoring to enum is pending - you can either do it here or in the follow-up PR where you extend for empty citations.
My suggestion would be to get it done - for now keep only NORMAL (or PARENTHETIC) and IN_TEXT.

@subhramit subhramit removed the status: awaiting-second-review For non-trivial changes label Mar 26, 2026
@pluto-han

Copy link
Copy Markdown
Collaborator Author

@pluto-han just missed that conversion to enum is pending - you can either do it here or in the follow-up PR where you extend for empty citations.刚刚错过了枚举类型的转换正在进行中——你可以在这里进行转换,也可以在后续的 PR 中进行转换,以扩展空引用。 My suggestion would be to get it done - for now keep only NORMAL (or PARENTHETIC) and IN_TEXT.我的建议是先完成这项工作——目前只保留 NORMAL(或 PARENTHETIC)和 IN_TEXT。

I think we can do it in a follow-up PR because empty citations are a bit different, as they can coexist with normal/intext ones (pls correct me if I am wrong). Hence inserting methods likely need a small refactor for empty citations instead of reusing the logic for normal/intext ones.

I will submit a PR very soon!

@subhramit

subhramit commented Mar 26, 2026

Copy link
Copy Markdown
Member

I think we can do it in a follow-up PR because empty citations are a bit different, as they can coexist with normal/intext ones (pls correct me if I am wrong). Hence inserting methods likely need a small refactor for empty citations instead of reusing the logic for normal/intext ones.

You can assume that only one type of citation will exist for a document. Otherwise the update/rewrite logic will get complex, especially for numeric citations.

Try to check how JStyles behaves with regard to it currently.

@subhramit

subhramit commented Mar 26, 2026

Copy link
Copy Markdown
Member

because empty citations are a bit different, as they can coexist with normal/intext ones

You are right about this, though.
The problem (or the fact) is that CSL citations are treated with much more uniformity. Invisible citations don't even follow those styles (except for the bibliography part) - it is otherwise supported as just a counterpart of JStyles with not many use cases.

@subhramit

Copy link
Copy Markdown
Member

I will submit a PR very soon!

Letting this in for now.
If possible, also create a follow-up issue to this one and link it to your coming PR.

@subhramit subhramit added this pull request to the merge queue Mar 27, 2026
@github-actions github-actions Bot added the status: to-be-merged PRs which are accepted and should go into the merge-queue. label Mar 27, 2026
Merged via the queue into JabRef:main with commit 96207af Mar 27, 2026
78 of 79 checks passed
@pluto-han

pluto-han commented Mar 27, 2026

Copy link
Copy Markdown
Collaborator Author

You can assume that only one type of citation will exist for a document.

Follow-up issue created.

Siedlerchr added a commit to geovani-rocha/jabref that referenced this pull request Mar 28, 2026
…o fix-group-icons

* 'fix-group-icons' of github.com:geovani-rocha/jabref: (26 commits)
  chore(deps): update dependency org.apache.logging.log4j:log4j-to-slf4j to v2.25.4 (JabRef#15436)
  chore(deps): update jackson monorepo to v3.1.1 (JabRef#15435)
  Fix PushToPreferences reset and import (JabRef#15395)
  Add fulltext fetcher for Wiley via their TDM API (JabRef#15388)
  Embed in-text nature in reference marks for CSL citations (JabRef#15381)
  Chore(deps): Bump com.gradleup.shadow:shadow-gradle-plugin (JabRef#15430)
  Fix not on fx thread exceptions for cleanup and cite key generator (JabRef#15424)
  Revert "Update gradle to nightly of 2026-03-23 (JabRef#15372)"
  feat: add benchmarks for Lucene fulltext search and linked file indexing, including setup and teardown of the index. (JabRef#15385)
  Chore(deps): Bump org.openrewrite.recipe:rewrite-recipe-bom (JabRef#15418)
  Add claude gitignore (JabRef#15413)
  Fix group filter icon in side pane (JabRef#15408)
  Add new prs_link feature
  Chore(deps): Bump org.glassfish.hk2:hk2-api in /versions (JabRef#15422)
  Chore(deps): Bump org.openrewrite.rewrite from 7.28.2 to 7.29.0 (JabRef#15419)
  Chore(deps): Bump jablib/src/main/resources/csl-styles (JabRef#15417)
  Fix for inconsistent "hide tab bar" behavior (JabRef#15409)
  Update dependency org.glassfish.hk2:hk2-utils to v4 (JabRef#15407)
  Persist file notifications (JabRef#15403)
  Update dependency org.glassfish.hk2:hk2-locator to v4 (JabRef#15405)
  ...
Ranjeet2702 pushed a commit to Ranjeet2702/jabref that referenced this pull request Apr 14, 2026
* Embed in-text nature in reference mark

* Fix test

* Delete redundant code for `inTextUsed`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component: libre-office status: no-bot-comments status: to-be-merged PRs which are accepted and should go into the merge-queue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enrich CSL Reference mark format with citation type information

3 participants