Skip to content

CSL4LibreOffice - G [Custom CSL Styles, build-time loading]#12951

Merged
calixtus merged 86 commits into
JabRef:mainfrom
subhramit:custom-csl
Apr 18, 2025
Merged

CSL4LibreOffice - G [Custom CSL Styles, build-time loading]#12951
calixtus merged 86 commits into
JabRef:mainfrom
subhramit:custom-csl

Conversation

@subhramit

@subhramit subhramit commented Apr 16, 2025

Copy link
Copy Markdown
Member

Custom CSL Styles & build-time loading

Follow-up to #12472

A. Functionality

  • The list of available CSL styles is now determined at build-time.
    Common metadata (path, title, isNumericStyle) from internal .csl files is pre-fetched and stored in a JSON file (citation-style-catalog.json) via CitationStyleCatalogGenerator. This reduces delays in population of the styles list (and cuts off some parsing overhead) for both the OO/LO integration as well as the preview viewer.

    Form:

{
  "path" : "academy-of-management-review.csl",
  "isNumeric" : false,
  "title" : "Academy of Management Review"
},
  • Revamped Select Style dialog UI - now both the CSL and JStyle tabs uniformly use TableView.

  • Support for external CSL styles:

    Screenshot 2025-04-17 205706

B. Code quality

  • Separation of concerns - CitationStyle now only handles modelling of a CSL style. CitationStyleCatalogGenerator along with CSLStyleLoader takes care of loading/removal of styles. CSLStyleUtils handles creation of CSL style instances and parsing.
  • Lot of refactoring & modularization of code (e.g. in StyleSelectDialogView, the handling of different components is now much more neatly compartmentalized for both CSL styles and JStyles. StyleSelectDialogViewModel has also been refactored for better manageability).
  • Complete renaming of OOBibStyle to JStyle.
  • Removal of unified style handling components from the JStyle class.

Closes #12337

Mandatory checks

  • I own the copyright of the code submitted and I license it under the MIT license
  • Change in CHANGELOG.md described in a way that is understandable for the average user (if change is visible to the user)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (if change is visible to the user)
  • Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
  • Checked 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 to the 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>
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>
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>
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>
@koppor

koppor commented Apr 18, 2025

Copy link
Copy Markdown
Member

(PR state when ticket-check-action does not work as expected)

koppor and others added 8 commits April 18, 2025 12:25
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>
FileDialogConfiguration fileDialogConfiguration = new FileDialogConfiguration.Builder()
.addExtensionFilter(Localization.lang("Style file"), StandardFileType.JSTYLE)
.withDefaultExtension(Localization.lang("Style file"), StandardFileType.JSTYLE)
.addExtensionFilter(Localization.lang("JStyle file"), StandardFileType.JSTYLE)

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.

Maybe %0 file? I'm sure we have it somewhere

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Added to localization.
I found:

this.description = Localization.lang("%0 file", fileType.getName());

So do you mean:

.addExtensionFilter(Localization.lang("%0 file", StandardFileType.JSTYLE.getName()), StandardFileType.JSTYLE)
.withDefaultExtension(Localization.lang("%0 file", StandardFileType.JSTYLE.getName()), StandardFileType.JSTYLE)

That causes %0 to be "LibreOffice layout style". I can change that to "JStyle" and use it.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Commit 45c7d51

Signed-off-by: subhramit <subhramit.bb@live.in>
@subhramit

Copy link
Copy Markdown
Member Author

Visual consistency for JStyles tab:

Comparison with CSL:

image
image

Old:

image

Comment on lines +98 to +102
if (matchingLayout.isEmpty()) {
matchingLayout = availableCslLayouts.stream()
.filter(layout -> layout.getDisplayName().equals(citationStyle.getTitle()))
.findFirst();
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The code uses an else branch to handle the case where matchingLayout is empty. It should return early if matchingLayout is present, following the fail-fast principle.

Comment on lines +193 to +194
path.map(Path::toAbsolutePath).map(Path::toString).ifPresent(stylePath -> {
Optional<CitationStyle> newStyleOptional = cslStyleLoader.addStyleIfValid(stylePath);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The code does not handle the case where path is empty before proceeding with the mapping operation. It should return early if path is not present.

Comment on lines +308 to +309
path.map(Path::toAbsolutePath).map(Path::toString).ifPresent(stylePath -> {
if (jStyleLoader.addStyleIfValid(stylePath)) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The code does not handle the case where path is empty before proceeding with the mapping operation. It should return early if path is not present.

@trag-bot

trag-bot Bot commented Apr 18, 2025

Copy link
Copy Markdown

@trag-bot didn't find any issues in the code! ✅✨

@calixtus calixtus added this pull request to the merge queue Apr 18, 2025
Merged via the queue into JabRef:main with commit f50e3e3 Apr 18, 2025
@calixtus calixtus deleted the custom-csl branch April 18, 2025 18:53
@JabRef JabRef deleted a comment from priyanshu16095 Apr 19, 2025
@subhramit subhramit removed the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Apr 19, 2025
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.

Load custom CSL style

5 participants