CSL4LibreOffice - F [Real-time switching of CSL Styles]

[subhramit]

Real-time switching of CSL Styles

Follow-up to #11712 and #12309
Closes #12309 (comment)
Closes https://github.com/JabRef/jabref-issue-melting-pot/issues/772 (internal)

Summary

• On selecting a new style, we no longer have to manually remove and re-cite the previously cited entries to update them. On citing the next entry, all other citations in the document will also be updated to have the currently selected style.
• Updates work within and across both in-text and non in-text citations.
• Multiple instances of CSLCitationOOAdapter are no longer created for each cite action. Only one instance throughout for a LibreOffice document connection.
• Bug fix: Formatting of numeric styles with tags such as <sup>5</sup> used to be lost due to direct UNO setText calls in the number reassignment feature. Now, OOTextIntoOO.write is used, which properly parses those tags to apply formatting.
• Better performance: CSL integration is now faster when interacting with the document. This was achieved changing the order of operations in certain methods and removing unnecessary UNO API calls/redundant interaction steps.
• A lot of refactoring - which also led to extraction of methods for which now tests could be added.

Limitations

• Works only for CSL styles

Mandatory checks

• I own the copyright of the code submitted and I licence 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 (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]

PR #12309 can be merged independent of this.

[subhramit]

@calixtus Architecture violation is due to the stateManager being used in the adapter (which is inside /logic).
I make a single instance of the adapter for one libreoffice connection in OOBibBase (which is inside /gui) - so if we were to pass the collected databases down via it, the architecture wouldn't be violated, but we will have to re-establish the document connection every time we add a new library to get the latest state of databases.

One thought was instead of constructor injection for the adapter, we could obtain the databases in OOBibBase itself via a separate function call to it - but that will create a reverse-dependency of OOBibBase in the adapter (which is below it in the hierarchy). I guess that would also violate the architecture as we'd still be using the instance of a gui class inside logic.

Making a new instance of the adapter (and passing latest databases via constructor) every time we add or remove a new library would be the architecturally correct solution, but it didn't make sense.
So this was the solution I stuck to.

[Siedlerchr]

You could maybe add a supplier callback to be able to get the state manager's list of database from a UI context

[calixtus]

The real question is, why in logic (business logic, alternating bibentries and so on) one has to get an observable list of currently open databases? The failing architecture test is just telling us, that thre is stuff that does not belong there. I need to take a deeper look. I can probably look tomorrow at it.

[subhramit]

The real question is, why in logic (business logic, alternating bibentries and so on) one has to get an observable list of currently open databases? The failing architecture test is just telling us, that thre is stuff that does not belong there.

That's because there is no method in /logic that gets us the freshest state of databases when called. We will need to bind it to a change listener if we want to access it indirectly while maintaining the gui/logic separation. Then, we can use this current "set" value of databases in logic.

I need to take a deeper look. I can probably look tomorrow at it.

Sure!

[HoussemNasri]

@subhramit please mark the review comments that you handled as resolved.