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

Clean up defintions of entry types #11013

Merged
merged 13 commits into from Mar 14, 2024
Merged

Clean up defintions of entry types #11013

merged 13 commits into from Mar 14, 2024

Conversation

koppor
Copy link
Member

@koppor koppor commented Mar 12, 2024

We have org.jabref.model.entry.types.BiblatexEntryTypeDefinitions, which defines all known BibLaTeX entry types. However, when diving into details, it is a WTF, because fields were added as important and as detail. Or as required and important. This was confusing.

This PR cleans up duplicate entries and keeps the more higher ranked (required > optional-important > optional-detail).

This also reorders to required/optionalImportant/optionalDetail in the code and adapts the names.

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.

@calixtus
Copy link
Member

Test is failing.

Im wondering it did not correspond with the specification in https://ftp.rrzn.uni-hannover.de/pub/mirror/tex-archive/macros/latex/contrib/biblatex/doc/biblatex.pdf before...

@koppor
Copy link
Member Author

koppor commented Mar 12, 2024

Test is failing.

Was green on Windows, but now on NixOS I can reproduce 🤣🤣

Im wondering it did not correspond with the specification in ftp.rrzn.uni-hannover.de/pub/mirror/tex-archive/macros/latex/contrib/biblatex/doc/biblatex.pdf before...

git blame game? 🤣 -- The distinction between importantOptional and detailedOptional was introduced at #5148.

@koppor
Copy link
Member Author

koppor commented Mar 12, 2024

image

@Siedlerchr
Copy link
Member

Siedlerchr commented Mar 12, 2024 via email

@koppor
Copy link
Member Author

koppor commented Mar 12, 2024

We have to be really careful here, when changing the fields around

I remove duplicate fields. - I wonder why it worked at all ^^. - JabRef did not display fields twice, because of the use of Set.

@koppor koppor enabled auto-merge March 12, 2024 22:40
@koppor koppor added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Mar 12, 2024
@Siedlerchr
Copy link
Member

conflicts in the javadoc, apart from that okay

private SequencedSet<OrFields> requiredFields = new LinkedHashSet<>();
private SequencedSet<BibField> optionalFields = new LinkedHashSet<>();

private Set<Field> seenFields = new HashSet<>();
Copy link
Member

Choose a reason for hiding this comment

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

What is seen fields now?

Copy link
Member Author

Choose a reason for hiding this comment

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

We are in the context of the Builder. The builder should raise an IllegalArgumentException of arguments are wrong. Therefore, the builder needs to know what it has seen.

this.fields = Streams.concat(fields.stream(), newFields.stream().map(field -> new BibField(field, FieldPriority.IMPORTANT)))
.collect(Collectors.toCollection(LinkedHashSet::new));
List<Field> containedFields = containedInSeenFields(newFields);
if (!containedFields.isEmpty()) {
Copy link
Member

Choose a reason for hiding this comment

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

Isn't the idea of a set to prevent duplicates. Why the check here

Copy link
Member Author

Choose a reason for hiding this comment

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

Because of "throw new IllegalArgumentException". To warn dumb callers.

Copy link
Contributor

github-actions bot commented Mar 14, 2024

The build for this PR is no longer available. Please visit https://builds.jabref.org/main/ for the latest build.

@koppor koppor added this pull request to the merge queue Mar 14, 2024
Merged via the queue into main with commit 5cb70df Mar 14, 2024
20 of 21 checks passed
@koppor koppor deleted the refine-bibentrytypes branch March 14, 2024 17:35
Siedlerchr added a commit to Frequinzy/jabref that referenced this pull request Mar 15, 2024
* upstream/main:
  Convert RemoveBracesFormatterTest to @ParameterizedTest (JabRef#11033)
  Update teaching.md
  Remove non-existing recipe (JabRef#11029)
  Update CSL styles (JabRef#11031)
  Clean up defintions of entry types (JabRef#11013)
  Fix log file path on Windows (JabRef#11028)
  Change to rolling logs (JabRef#11023)
Siedlerchr added a commit that referenced this pull request Mar 17, 2024
* upstream/main: (26 commits)
  Speed up failure reporting (#11030)
  Importing of BibDesk Groups and Linked Files (#10968)
  Convert RemoveBracesFormatterTest to @ParameterizedTest (#11033)
  Update teaching.md
  Remove non-existing recipe (#11029)
  Update CSL styles (#11031)
  Clean up defintions of entry types (#11013)
  Fix log file path on Windows (#11028)
  Change to rolling logs (#11023)
  chore: remove repetitive words (#11015)
  Fix test names (#11014)
  Remove obsolete "Comments" tab configuration (#11011)
  Fix "Other fields" tab respecting custom tabs (#11012)
  [WIP] Extract PDF References (#10437)
  Fixed jump to entry from crossref (#11009)
  fix suggestion provider for crossref field (#10962)
  Use SequencedSet for required and optional fields (#11007)
  Bump io.github.classgraph:classgraph from 4.8.165 to 4.8.168 (#11005)
  Bump org.glassfish.hk2:hk2-api from 3.0.6 to 3.1.0 (#11006)
  Bump org.apache.logging.log4j:log4j-to-slf4j from 2.23.0 to 2.23.1 (#11003)
  ...

# Conflicts:
#	src/main/resources/csl-styles
Siedlerchr added a commit that referenced this pull request Mar 17, 2024
* upstream/main: (26 commits)
  Speed up failure reporting (#11030)
  Importing of BibDesk Groups and Linked Files (#10968)
  Convert RemoveBracesFormatterTest to @ParameterizedTest (#11033)
  Update teaching.md
  Remove non-existing recipe (#11029)
  Update CSL styles (#11031)
  Clean up defintions of entry types (#11013)
  Fix log file path on Windows (#11028)
  Change to rolling logs (#11023)
  chore: remove repetitive words (#11015)
  Fix test names (#11014)
  Remove obsolete "Comments" tab configuration (#11011)
  Fix "Other fields" tab respecting custom tabs (#11012)
  [WIP] Extract PDF References (#10437)
  Fixed jump to entry from crossref (#11009)
  fix suggestion provider for crossref field (#10962)
  Use SequencedSet for required and optional fields (#11007)
  Bump io.github.classgraph:classgraph from 4.8.165 to 4.8.168 (#11005)
  Bump org.glassfish.hk2:hk2-api from 3.0.6 to 3.1.0 (#11006)
  Bump org.apache.logging.log4j:log4j-to-slf4j from 2.23.0 to 2.23.1 (#11003)
  ...

# Conflicts:
#	src/main/resources/csl-styles
Siedlerchr added a commit to ror3d/jabref that referenced this pull request Mar 18, 2024
…link

* upstream/main:
  Bump org.apache.pdfbox:fontbox from 3.0.1 to 3.0.2 (JabRef#11042)
  Bump com.dlsc.gemsfx:gemsfx from 2.2.0 to 2.4.0 (JabRef#11044)
  Bump org.apache.pdfbox:xmpbox from 3.0.1 to 3.0.2 (JabRef#11041)
  Bump com.googlecode.plist:dd-plist from 1.23 to 1.28 (JabRef#11040)
  Bump gittools/actions from 0.13.4 to 1.1.1 (JabRef#11039)
  Change copy-paste function to handle string constants (follow up PR) (JabRef#11037)
  Fixes Zotero file handling for absolute paths (JabRef#11038)
  Speed up failure reporting (JabRef#11030)
  Importing of BibDesk Groups and Linked Files (JabRef#10968)
  Convert RemoveBracesFormatterTest to @ParameterizedTest (JabRef#11033)
  Update teaching.md
  Remove non-existing recipe (JabRef#11029)
  Update CSL styles (JabRef#11031)
  Clean up defintions of entry types (JabRef#11013)
  Fix log file path on Windows (JabRef#11028)
  Change to rolling logs (JabRef#11023)
  chore: remove repetitive words (JabRef#11015)
  Fix test names (JabRef#11014)
Siedlerchr added a commit to Anirudhxx/jabref that referenced this pull request Mar 18, 2024
* upstream/main: (40 commits)
  Improve citation relations (JabRef#11016)
  Keep enclosing braces of authors (JabRef#11034)
  Bump org.apache.pdfbox:fontbox from 3.0.1 to 3.0.2 (JabRef#11042)
  Bump com.dlsc.gemsfx:gemsfx from 2.2.0 to 2.4.0 (JabRef#11044)
  Bump org.apache.pdfbox:xmpbox from 3.0.1 to 3.0.2 (JabRef#11041)
  Bump com.googlecode.plist:dd-plist from 1.23 to 1.28 (JabRef#11040)
  Bump gittools/actions from 0.13.4 to 1.1.1 (JabRef#11039)
  Change copy-paste function to handle string constants (follow up PR) (JabRef#11037)
  Fixes Zotero file handling for absolute paths (JabRef#11038)
  Speed up failure reporting (JabRef#11030)
  Importing of BibDesk Groups and Linked Files (JabRef#10968)
  Convert RemoveBracesFormatterTest to @ParameterizedTest (JabRef#11033)
  Update teaching.md
  Remove non-existing recipe (JabRef#11029)
  Update CSL styles (JabRef#11031)
  Clean up defintions of entry types (JabRef#11013)
  Fix log file path on Windows (JabRef#11028)
  Change to rolling logs (JabRef#11023)
  chore: remove repetitive words (JabRef#11015)
  Fix test names (JabRef#11014)
  ...
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.

None yet

3 participants