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

CSL4LibreOffice - E [Add re-distribution of numeric CSL citations] #11712

Merged
merged 54 commits into from
Sep 7, 2024

Conversation

subhramit
Copy link
Collaborator

@subhramit subhramit commented Sep 6, 2024

Fixes subhramit#22
Follow-up to #11521, #11577

  • Proper sync of citation numbers throughout the document
  • Refactoring (for consistent conventions, removal of redundant conditions, etc.)
  • Documentation updates
sameLineTest.mp4
multiLineTest.mp4
groups.mp4

Limitations:

  1. Fix does not work for grouped citations (it is a more difficult problem, refs. [WIP] CSL citations: Fix sync numbering #11688) (Solved)
  2. Can be broken by moving a reference mark within the boundary of another, since there is no safety implemented for "cursor in protected area" as present in JStyles.

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
Copy link
Collaborator Author

subhramit commented Sep 6, 2024

@koppor @Siedlerchr Please merge this before 6.0-alpha release.

@subhramit subhramit added this to the 6.0-alpha milestone Sep 6, 2024
ThiloteE
ThiloteE previously approved these changes Sep 7, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 7, 2024
@Siedlerchr Siedlerchr added this pull request to the merge queue Sep 7, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 7, 2024
@ThiloteE
Copy link
Member

ThiloteE commented Sep 7, 2024

I tested this PR. The only way that I found how to destroy its functionality was to select a numeric CSL style, to cite / cite-in-text and then to select the default numeric JStyle and cite / cite-in-text and then to delete some of the priorly cited references. Then pressed "Make/Sync Bibliography". This caused the JStyle reference list at the end of the document to update, but the CSL Style reference list to remain unchanged.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Your code currently does not meet JabRef's code guidelines.
We use OpenRewrite to ensure "modern" Java coding practices.
The issues found can be automatically fixed.
Please execute the gradle task rewriteRun, check the results, commit, and push.

You can check the detailed error output by navigating to your pull request, selecting the tab "Checks", section "Tests" (on the left), subsection "OpenRewrite".

@Siedlerchr Siedlerchr dismissed stale reviews from ThiloteE and themself via 9db142c September 7, 2024 12:24
@Siedlerchr Siedlerchr added this pull request to the merge queue Sep 7, 2024
Merged via the queue into JabRef:main with commit bcbe126 Sep 7, 2024
22 checks passed
@Siedlerchr Siedlerchr deleted the numberings-2 branch September 7, 2024 12:42
@subhramit
Copy link
Collaborator Author

subhramit commented Sep 7, 2024

I tested this PR. The only way that I found how to destroy its functionality was to select a numeric CSL style, to cite / cite-in-text and then to select the default numeric JStyle and cite / cite-in-text and then to delete some of the priorly cited references. Then pressed "Make/Sync Bibliography". This caused the JStyle reference list at the end of the document to update, but the CSL Style reference list to remain unchanged.

Hey, thanks for trying out. Yes, the jstyle and csl style citation detection, parsing, bibliography section population and syncing pipelines are completely independent of each other and can't be (or rather isn't recommended to be) used together (one of the reasons being the reference mark format being different - Jstyles don't start with "JABREF_" as you may have noticed with csl). After working with jstyles, if you switch back to a csl style and press refresh, the bibliography for csl would update again, keeping the jstyle section independent in that case (i mean to say behavior is symmetrical the other way around as well).

@subhramit subhramit restored the numberings-2 branch September 7, 2024 16:45
@ThiloteE
Copy link
Member

ThiloteE commented Sep 7, 2024

I tested this PR. The only way that I found how to destroy its functionality was to select a numeric CSL style, to cite / cite-in-text and then to select the default numeric JStyle and cite / cite-in-text and then to delete some of the priorly cited references. Then pressed "Make/Sync Bibliography". This caused the JStyle reference list at the end of the document to update, but the CSL Style reference list to remain unchanged.

Hey, thanks for trying out. Yes, the jstyle and csl style citation detection, parsing, bibliography section population and syncing pipelines are completely independent of each other and can't be (or rather isn't recommended to be) used together (one of the reasons being the reference mark format being different - Jstyles don't start with "JABREF_" as you may have noticed with csl). After working with jstyles, if you switch back to a csl style and press refresh, the bibliography for csl would update again, keeping the jstyle section independent in that case (i mean to say behavior is symmetrical the other way around as well).

I think it would be good to add this piece of information to the documentation. It is an edge case and not many people will use both csl and jstyle in one document, but when they do, they will run into this.

@subhramit
Copy link
Collaborator Author

subhramit commented Sep 7, 2024

I tested this PR. The only way that I found how to destroy its functionality was to select a numeric CSL style, to cite / cite-in-text and then to select the default numeric JStyle and cite / cite-in-text and then to delete some of the priorly cited references. Then pressed "Make/Sync Bibliography". This caused the JStyle reference list at the end of the document to update, but the CSL Style reference list to remain unchanged.

Hey, thanks for trying out. Yes, the jstyle and csl style citation detection, parsing, bibliography section population and syncing pipelines are completely independent of each other and can't be (or rather isn't recommended to be) used together (one of the reasons being the reference mark format being different - Jstyles don't start with "JABREF_" as you may have noticed with csl). After working with jstyles, if you switch back to a csl style and press refresh, the bibliography for csl would update again, keeping the jstyle section independent in that case (i mean to say behavior is symmetrical the other way around as well).

I think it would be good to add this piece of information to the documentation. It is an edge case and not many people will use both csl and jstyle in one document, but when they do, they will run into this.

Good idea. On my list.
We also need to add the information on how to make reference marks visible in the user documentation.
https://github.com/JabRef/user-documentation (Keeping this here)

@ThiloteE
Copy link
Member

ThiloteE commented Sep 7, 2024

Yes, I opened an issue here for the CSL additions: JabRef/user-documentation#509.
Instead of collecting this stuff in pull-requests, it would be better to open a PR to the docs directly and collect there or at least collect data that requires changes to documentation in the issue I linked.

@subhramit subhramit changed the title [IMP] Add re-distribution of numeric CSL citations CSL4LibreOffice - E [Add re-distribution of numeric CSL citations] Sep 9, 2024
@subhramit subhramit changed the title CSL4LibreOffice - E [Add re-distribution of numeric CSL citations] CSL4LibreOffice - E: Add re-distribution of numeric CSL citations Sep 9, 2024
@subhramit subhramit changed the title CSL4LibreOffice - E: Add re-distribution of numeric CSL citations CSL4LibreOffice - E [Add re-distribution of numeric CSL citations] Sep 9, 2024
leaf-soba pushed a commit to leaf-soba/jabref that referenced this pull request Sep 11, 2024
* Initial commit

* Semi-fix: 0-based index

* add method for getting multiple citations

* Use XTextRangeCompare

* Fix update order

* Remove dead code, undo ai refactor changes

* Fix remaining order issues, Remove dead code, undo ai refactor changes

* Restore exceptions: UpdateCSLBibliography

* Refine

* Fix checkstyle

* Remove experiment

* Fix OOBibBase.java exceptions

* Fix OpenOfficePanel.java exceptions

* Fix adapter exception indent

* Fix adapter exception order

* Restore javadoc

* Restore javadoc, remove dead code

* Remove dead code

* Improve implementation (general changes)

* Change printStackTrace->Logger

* Fix readExistingMarks()

* Remove logs

* Finish CSLReferenceMarkManager

* openRewrite

* fix unit test (std streams -> logger)

* Redundant log

* Safety, refactoring

* Add log

* Better log

* Change log level (non-breaking)

* Add handling for multiple entries

* Better log

* Restore javadoc

* Restore javadoc

* Use list instead of singleton

* Adapt test

* Add comment

* Add stricter conditions for update

* Add stricter conditions for reading

* Make xtextcompare final

* Null handling

* Update javadoc

* Refactor

* Remove redundant condition

* Fix non-numeric side effect

* Enhance implementation

* Add newline

* Bug fix: no sync in refresh bibliography

* fix logger openrewrite

---------

Co-authored-by: Siedlerchr <siedlerkiller@gmail.com>
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.

Make/Sync bibliography should remove entries
3 participants