Skip to content

Make CSL Citation Adapter non-static#11535

Merged
Siedlerchr merged 5 commits into
JabRef:mainfrom
subhramit:oo-architecture-3
Jul 30, 2024
Merged

Make CSL Citation Adapter non-static#11535
Siedlerchr merged 5 commits into
JabRef:mainfrom
subhramit:oo-architecture-3

Conversation

@subhramit

@subhramit subhramit commented Jul 25, 2024

Copy link
Copy Markdown
Member

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 dev: 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
Copy Markdown
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
Copy Markdown
Member 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
Copy Markdown
Member Author

Yay fixed submodules myself this time :)

@subhramit

Copy link
Copy Markdown
Member Author

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

@Siedlerchr Siedlerchr enabled auto-merge July 30, 2024 10:37
@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
@Siedlerchr Siedlerchr deleted the oo-architecture-3 branch July 30, 2024 10:47
* 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
Copy Markdown
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
Copy Markdown
Member 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

component: libre-office dev: code-quality Issues related to code or architecture decisions project: gsoc

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants