Persist citation nature in the CSL reference mark#15437
Open
pluto-han wants to merge 4 commits intoJabRef:mainfrom
Open
Persist citation nature in the CSL reference mark#15437pluto-han wants to merge 4 commits intoJabRef:mainfrom
pluto-han wants to merge 4 commits intoJabRef:mainfrom
Conversation
pluto-han
changed the title
Contributor
Review Summary by QodoPersist citation type in CSL reference marks using enum
WalkthroughsDescription• Replace boolean inText variable with CSLCitationType enum • Embed citation type (NORMAL/IN_TEXT/EMPTY) into CSL reference marks • Update reference mark format to persist citation nature across document operations • Refactor citation type tracking throughout adapter and manager classes Diagramflowchart LR
A["Boolean inText"] -->|Replace with| B["CSLCitationType Enum"]
B -->|NORMAL| C["Reference Mark Format"]
B -->|IN_TEXT| C
B -->|EMPTY| C
C -->|Embedded in| D["Citation Persistence"]
File Changes1. jablib/src/main/java/org/jabref/logic/openoffice/oocsltext/CSLCitationType.java
|
Contributor
Code Review by Qodo
1. Changelog entry too technical
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
|
|
||
| ### Changed | ||
|
|
||
| - We changed CSL reference format by adding citation type at the end, and used enum to replace boolean variables in the method signature. [#15370](https://github.com/JabRef/jabref/issues/15370)[#15434](https://github.com/JabRef/jabref/issues/15434) |
Contributor
There was a problem hiding this comment.
1. Changelog entry too technical 📘 Rule violation ⚙ Maintainability
The new CHANGELOG.md entry includes developer/internal implementation details (enum/boolean method signature) and has inconsistent issue link formatting (missing space between references). This is not end-user oriented and reduces release note readability.
Agent Prompt
## Issue description
The new changelog line is written in developer-focused terms and the issue links are not consistently formatted (missing spacing), reducing end-user readability.
## Issue Context
Changelog entries should describe user-visible behavior changes in clear language and keep Markdown links consistently formatted.
## Fix Focus Areas
- CHANGELOG.md[28-28]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
jablib/src/main/java/org/jabref/logic/openoffice/oocsltext/CSLReferenceMarkManager.java
Outdated
Show resolved
Hide resolved
Comment on lines
148
to
152
| CSLReferenceMark mark = new CSLReferenceMark(named, referenceMark); | ||
| marksByName.put(name, mark); | ||
| marksInOrder.add(mark); | ||
| isInTextCite = isInTextCite || referenceMark.isInText(); | ||
| citationType = referenceMark.getCitationType(); | ||
|
|
Contributor
There was a problem hiding this comment.
3. Citation type derived from last 🐞 Bug ✓ Correctness
CSLReferenceMarkManager.readAndUpdateExistingMarks overwrites the manager-wide citationType for each mark and ends up returning the last mark’s type. If a document contains mixed/legacy marks, CSLCitationOOAdapter can skip required global updates or apply the wrong global conversion, leaving mixed citation types in the document.
Agent Prompt
### Issue description
`readAndUpdateExistingMarks` sets `citationType` to the last mark’s type, which can be wrong when marks differ (common for legacy docs or corrupted state) and causes the adapter’s global-update decision to be incorrect.
### Issue Context
The adapter relies on a document-wide citation type to decide whether it must rewrite all existing citations before inserting a new one.
### Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/openoffice/oocsltext/CSLReferenceMarkManager.java[129-163]
- jablib/src/main/java/org/jabref/logic/openoffice/oocsltext/CSLCitationOOAdapter.java[57-71]
- jablib/src/main/java/org/jabref/logic/openoffice/oocsltext/CSLCitationOOAdapter.java[80-100]
### Suggested fix
Compute document citationType by aggregating across all marks (e.g., if any EMPTY -> EMPTY, else if any IN_TEXT -> IN_TEXT, else NORMAL), and optionally log a warning if multiple types are found. Ensure the adapter refreshes its `citationType` from the manager after `readAndUpdateExistingMarks()` in workflows that depend on the current document state.
ⓘ 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.
subhramit
reviewed
Mar 28, 2026
| // Current citation type in the doc | ||
| private CSLCitationType citationType; | ||
| // True when user inserts a citation with different type | ||
| private boolean citationTypeIsChanged; |
Member
There was a problem hiding this comment.
Variable names are sufficiently descriptive - so the comments seem superfluous.
Comment on lines
+179
to
+180
| String citationType = parts[parts.length - 1]; | ||
| int uniqueIdIndex = parts.length - 2; |
Member
There was a problem hiding this comment.
Add some field-level javadoc with a reference mark example, so that the numbers easily make sense when reading code.
Comment on lines
+79
to
+80
| default -> | ||
| CSLCitationType.NORMAL; |
Member
There was a problem hiding this comment.
The negative side of putting this in a default case is that (hypothetically), a refrence mark JABREF_Keen_2011 CID_1 xyzuzuw1u FOOBAR would also be parsed successfully in this case - which is not exactly a good/"strong" programming practice.
The positive side is that this will allow for backward compatibility.
I think the positive outweighs here, so it's fine.
Contributor
Author
There was a problem hiding this comment.
a refrence mark
JABREF_Keen_2011 CID_1 xyzuzuw1u FOOBARwould also be parsed successfully in this case
Can reference mark with different citation type (other than intext/empty/normal) be created?
✅ All tests passed ✅🏷️ Commit: 88abbdb Learn more about TestLens at testlens.app. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related issues and pull requests
Closes #15434
PR Description
This PR embeded citation nature into reference marks, and use enum to replace boolean variables.
Steps to test
Checklist
CHANGELOG.mdin a way that can be understood by the average user (if change is visible to the user)