Embed in-text nature in reference marks for CSL citations#15381
Conversation
Review Summary by QodoAdd in-text citation marker support to reference marks
WalkthroughsDescription• 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 Diagramflowchart 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"]
File Changes1. jablib/src/main/java/org/jabref/logic/openoffice/ReferenceMark.java
|
Code Review by Qodo
1. parts[1] accessed unsafely
|
| 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(); |
There was a problem hiding this comment.
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
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
|
||
| OOText ooText = OOFormat.setLocaleNone(OOText.fromString(formattedCitation)); | ||
| insertReferences(cursor, entries, ooText, isNumericStyle); | ||
| insertReferences(cursor, entries, ooText, isNumericStyle, false); |
There was a problem hiding this comment.
boolean could be replaced with an enum for better readability.
There was a problem hiding this comment.
@subhramit Just for more suggestions, should we replace true/false with enum?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Yeah a enum makes more sense in this case, I was misunderstanding. Thank you! @subhramit @calixtus
There was a problem hiding this comment.
On first look, the direction and implementation seems correct. Good work,
I have two suggestions (apart from the separate comments above) -
- 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).
- 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)
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
Yes. From my tests, "insert empty citation" has the same problem with #15369. I will fix it in a follow-up PR. |
✅ All tests passed ✅🏷️ Commit: 8df57fa Learn more about TestLens at testlens.app. |
|
@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. |
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! |
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. |
You are right about this, though. |
Letting this in for now. |
Follow-up issue created. |
…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) ...
* Embed in-text nature in reference mark * Fix test * Delete redundant code for `inTextUsed`
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
Restart JabRef and sync bibliography -> Citations refreshed to non-in-text
Now:

Remain in-text citations
Checklist
CHANGELOG.mdin a way that can be understood by the average user (if change is visible to the user)