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

Fix the representation of season in Biblatex date field. #12537

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

urlyy
Copy link

@urlyy urlyy commented Feb 21, 2025

Fixes #12437

Description

When adding an entry like @Article{Article, date = {2025-21} }, the value 21 is interpreted as spring and added to the yeardivision entry field, rather than being treated as an out-of-range month and throwing an exception.

For reference:
image

Demonstration

image

Existing problems

  • Should I use YearDivision or Season as the class name? The BibLaTeX documentation uses YearDivision, but it seems that YearDivision might also include some other values.
    image
  • Season does not require a shortName, is the getJabRefFormat method still necessary?

TODO

Since I am a beginner in bibliography management and not certain whether my approach is correct, I have only implemented it preliminarily. If you think it is acceptable, I will proceed to finish the following TODO items:

  • The handling for the format 2002-21 has been implemented, but the handling for the format 2002-21/2002-23 has not yet been implemented.
    image
  • Remove the redundant parts in the Season class.
  • Improve the basic functions of the Date class, such as toString, equals, and hashCode

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.

@urlyy urlyy changed the title Fix #12437 Fix the representation of seasons in Biblatex date field. Feb 21, 2025
@urlyy urlyy changed the title Fix the representation of seasons in Biblatex date field. Fix the representation of season in Biblatex date field. Feb 21, 2025
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.

JUnit tests are failing. In the area "Some checks were not successful", locate "Tests / Unit tests (pull_request)" and click on "Details". This brings you to the test output.
You can then run these tests in IntelliJ to reproduce the failing tests locally. We offer a quick test running howto in the section Final build system checks in our setup guide.

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.

JUnit tests are failing. In the area "Some checks were not successful", locate "Tests / Unit tests (pull_request)" and click on "Details". This brings you to the test output.
You can then run these tests in IntelliJ to reproduce the failing tests locally. We offer a quick test running howto in the section Final build system checks in our setup guide.

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.

JUnit tests are failing. In the area "Some checks were not successful", locate "Tests / Unit tests (pull_request)" and click on "Details". This brings you to the test output.
You can then run these tests in IntelliJ to reproduce the failing tests locally. We offer a quick test running howto in the section Final build system checks in our setup guide.

@Siedlerchr
Copy link
Member

Very good work! I would like to see some more tests, but otherwise on the first look it does a good first impression.

@Siedlerchr Siedlerchr added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Feb 21, 2025
@koppor koppor marked this pull request as ready for review February 25, 2025 14:55
Copy link
Member

@koppor koppor left a comment

Choose a reason for hiding this comment

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

Some minor comments.

Please also add a CHANGELOG.md entry

Comment on lines +370 to +373
if (season == null) {
return Optional.empty();
}
return Optional.of(season);
Copy link
Member

Choose a reason for hiding this comment

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

Please use Optional.ofNullable

Comment on lines +407 to +412
if (season != null) {
return "Date{" +
"date=" + formattedDate + ", " +
"season=" + season +
'}';
}
Copy link
Member

Choose a reason for hiding this comment

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

Why a different format then the others?

Isn't there a compact form (similar to DateTimeFormatter.ISO_DATE_TIME.format or other results here?)

Action point: Either use a compact form or add a Java comment that there is no compact form

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BibLaTeX seasons cause exception on import and export
3 participants