Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make CSL Citation Adapter non-static #11535

Merged
merged 5 commits into from
Jul 30, 2024

Conversation

subhramit
Copy link
Collaborator

@subhramit subhramit commented Jul 25, 2024

Making CSL Citation Adapter non-static

Follow-up to #11521

  • As per @calixtus 's comments on CSLCitationOOAdapter.java, we are making a small architectural change to make it's methods non-static. The static nature happened to be unnecessary, and was a result of trying to use dependencies in some way or the other to get the initial implementation of the adapter working.
  • Methods of the class that were unnecessarily public (and would not be used elsewhere) have also been made private.
  • Some necessary parameters are now passed down by constructor injection, instead of using new instances.
  • Also added suggested href tag to link in JavDoc of the transformHtml method.

Mandatory checks

  • Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (for UI changes)
  • 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.

@subhramit subhramit added the type: code-quality Issues related to code or architecture decisions label Jul 25, 2024
@subhramit subhramit mentioned this pull request Jul 25, 2024
6 tasks
private static final BibEntryTypesManager BIB_ENTRY_TYPES_MANAGER = new BibEntryTypesManager();
private static final CitationStyleOutputFormat FORMAT = CitationStyleOutputFormat.HTML;
private final CitationStyleOutputFormat format = CitationStyleOutputFormat.HTML;
private final BibEntryTypesManager bibEntryTypesManager = new BibEntryTypesManager();
Copy link
Member

Choose a reason for hiding this comment

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

Use the already existing instance either by BibEntryTypesManager entryTypesManager = Injector.instantiateModelOrService(BibEntryTypesManager.class);

or try to pass it down via constructor

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done. Went with the latter.

@subhramit
Copy link
Collaborator Author

Yay fixed submodules myself this time :)

@subhramit
Copy link
Collaborator Author

@Siedlerchr Any more changes needed, Chris? Or can this be merged?

@Siedlerchr Siedlerchr added this pull request to the merge queue Jul 30, 2024
Merged via the queue into JabRef:main with commit 622de99 Jul 30, 2024
21 checks passed
@Siedlerchr Siedlerchr deleted the oo-architecture-3 branch July 30, 2024 10:47
@@ -59,11 +58,11 @@ public static void writeCitation(XTextDocument doc, XTextCursor cursor, String c
* The transformed HTML can be used for inserting into a LibreOffice document
* Context: The HTML produced by CitationStyleGenerator.generateCitation(...) is not directly (completely) parsable by OOTextIntoOO.write(...)
* For more details, read the documentation of the write(...) method in the {@link OOTextIntoOO} class.
* Additional information: https://devdocs.jabref.org/code-howtos/openoffice/code-reorganization.html.
* Additional information: <a href="https://devdocs.jabref.org/code-howtos/openoffice/code-reorganization.html">...</a>.
Copy link
Member

Choose a reason for hiding this comment

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

This is wrong. Either repeat the link inside the a tag or use "additional information" instead of ...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Should I make a separate PR or club it with PR-C of my project?
I accepted IntelliJ's suggestion blindly, forgetting how html hrefs work🤦🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
openoffice/libreoffice project: GSoC type: code-quality Issues related to code or architecture decisions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants