-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Bugfix/5045 : Modified the existing logic to comply crossref resolution with biblatex specification #5086
Conversation
Merging from head master
Merging from head master
…n with biblatex specification
Thanks for your contribution. Could you please add a test for the changed code? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution. With your changes, the value of the field in the crossreferenced entry is always taken, even if it specified in the entry itself. This is not the problem descried in #5045 and not according to the biblatex specification.
The real problem is that biblatex specifies a more complex crossref resolution. I think a proper fix should go along the following lines:
if (!result.isPresent() && (database != null)) {
Optional<BibEntry> referred = database.getReferencedEntry(this);
if (!referred.isEmpty()) {
result = referred.get()..getFieldOrAlias(field);
if (result.isEmpty()) {
if (type == InProceedings && field = FieldNames.booktitle) {
result = referred.get().getFiledOrAlias(FieldName.title);
}
// Add other special cases that might be mentioned in the biblatex documentation
}
}
}
@tobiasdiez @Siedlerchr sure I'll make those amendments and the UTs |
Merging from Master
@tobiasdiez Could you please review the latest commit. Thanks :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me now.
Please merge the latest master changes in. There have been renaming of the Fields classes, so you need to fix this. Then we can merge
Merging from master
Merging from master
@Siedlerchr I pulled the changes from master. This PR can be merged to the master now. Thanks |
@@ -21,6 +21,8 @@ | |||
import javafx.collections.FXCollections; | |||
import javafx.collections.ObservableMap; | |||
|
|||
import org.jabref.logic.importer.fetcher.CrossRef; | |||
import org.jabref.logic.importer.fileformat.bibtexml.Inproceedings; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please check your imports, it seems like you have imported the type from the wrong package.
Must be import org.jabref.model.entry.StandardEntryType;
And CrossRef here looks also odd.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please review the imports.
Fixed and organized imports. Please review and merge |
Please have a look at travis checkstyle output. Star imports are not allowed. If you then run organize imports in the IDE it will be correct |
Fixed imports issue. Please review |
* upstream/master: Bugfix/5045 : Modified the existing logic to comply crossref resolution with biblatex specification (JabRef#5086) Fix issue with missing year value in year column (JabRef#5197) Bump com.gradle.build-scan from 2.4 to 2.4.1 (JabRef#5199) Add citation commands to TexParser: autocite, blockcquote, and textcquote (JabRef#5195) Conversion of preferencesDialog/advancedTab, networkTab and groupsTab to mvvm (JabRef#5141) Fix Permissions of LaTeXintegration (JabRef#5134) Border for group color indicator and some space for tooltip (JabRef#5190) Fix issue 5152, tooltip and icon added to group cell (JabRef#5191) Fix tooltips in CitationsDisplay (JabRef#5188)
a20406d Added name of the editors of a given edition (#5140) 9881fc5 Ping on push, not PR, document role of dist-updater (#5137) 04668cc Create nouvelles-perspectives-en-sciences-sociales.csl (#5063) 1d94e21 Update bursa-uludag-universitesi-saglik-bilimleri-enstitusu.csl (#5047) 84f3893 Add Harvard style for Metropolia University of Applied Sciences (#5086) 8e43e79 Create opto-electronic-advances.csl (#5135) 36e4fba Update society-for-american-archaeology.csl (#5124) 69ca360 St. Paul Canon Law new style (#5138) b490ab0 Update and rename st-paul-university-faculty-of-canon-law.csl to saint-paul-university-faculty-of-canon-law.csl b498116 There is no en-CA locale 3c35f28 Metadata 7059cca Create tu-dortmund-agvm.csl (#5088) c321c98 Create new Citation type (#5093) a7edc8d Update international-organization.csl (#5103) 3d1a052 The AWS load balancer is messing things up (#5133) ca3839b Fix sort by a single macro (#5136) 5d1a7e8 Update chungara-revista-de-antropologia-chilena.csl (#5123) cd75d5d ping distribution-updater (#5132) dcf473a Update wirtschaftsuniversitat-wien-health-care-management.csl (#5125) a87085e Fix Harvard Praxisforschung Editors (#5130) d4176ca Switch automated tests to Github Actions (#5111) 726d0d8 Radiology, MPP, CORR -- small fixes: https://forums.zotero.org/discussion/85883/doi-radiology#latest https://forums.zotero.org/discussion/51058/style-request-molecular-plant-pathology#latest https://forums.zotero.org/discussion/85678/citing-style-clinical-orthopaedics-and-related-research#latest e23db68 Update to la-trobe-university-harvard style (#5119) c54b278 Create wirtschaftsuniversitat-wien-health-care-management.csl (#5110) 62fb019 Create austral-entomology.csl (#5118) afa328c Update iso690-author-date-en.csl (#5113) 5468dce Update iso690-author-date-fr-no-abstract.csl (#5112) 98af86c Update iso690-numeric-fr.csl (#5115) 09f84c4 Update iso690-author-date-fr.csl (#5114) 178a9e4 Fix current biology to superscript 1fa5ce7 Create droit-belge-centre-de-droit-prive-ulb.csl (#5107) 3a6a4bc Fix file modes (#5106) 48f50e5 Create chungara-revista-de-antropologia-chilena.csl (#5096) 1e848f8 Create the-journal-of-the-indian-law-institute.csl (#5100) 856524c Create molecular-biology.csl (#5101) eeebbf4 Create harvard-harper-adams-university.csl (#5104) d90993d Fix tests d4037bf WIP: St Paul Canon Law style git-subtree-dir: src/main/resources/csl-styles git-subtree-split: a20406d
Fixes #5045