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 CSL rendering in case of article #8607

Merged
merged 23 commits into from
Aug 30, 2022
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
69ac610
Rewrite to parameterized tests
koppor Mar 7, 2022
0b91011
Fix output format of CSL could not be changed if new style was selected
koppor Mar 7, 2022
a2a76d6
Add test cases
koppor Mar 26, 2022
0354542
Merge remote-tracking branch 'origin/main' into fix-8372
koppor Mar 28, 2022
d148a54
Work on CitationSTyleGeneratorTest (and fix PagesChecker)
koppor Mar 28, 2022
32fb34e
Refine special pages handling
koppor Mar 28, 2022
a7b299d
Merge remote-tracking branch 'origin/main' into fix-8372
koppor Apr 25, 2022
cfb2073
Rewrite PagesCheckerTest to ParameterizedTest
koppor Apr 25, 2022
c579d36
Craft out PagesCheckerTest for BibLaTeX
koppor Apr 25, 2022
3ac0daa
Some updaes
koppor Apr 25, 2022
4283ebe
Experimenting with more
koppor Apr 25, 2022
e7374f9
Merge remote-tracking branch 'upstream/main' into fix-8372
Siedlerchr Aug 6, 2022
c155402
Fixed rest of tests; mapped eid to page
ThiloteE Aug 10, 2022
b9ae73f
Merge branch 'fix-8372' of https://github.com/JabRef/jabref into fix-…
ThiloteE Aug 10, 2022
41fa5ad
Fix checkstyle
ThiloteE Aug 10, 2022
79dcc16
Update CHANGELOG.md
calixtus Aug 11, 2022
20ba810
Map biblatex eid to csl number
ThiloteE Aug 30, 2022
c028c46
Merge branch 'fix-8372' of https://github.com/JabRef/jabref into fix-…
ThiloteE Aug 30, 2022
27434c6
Merge branch 'main' into fix-8372
Siedlerchr Aug 30, 2022
538dc15
Fix checkstyle
ThiloteE Aug 30, 2022
1727fda
Fix checkstyle again
ThiloteE Aug 30, 2022
25acf39
Fix default CitationStyleTest (IEEE)
ThiloteE Aug 30, 2022
4fa4243
Add changelog entry
ThiloteE Aug 30, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
- We fixed an issue where opening the changelog from withing JabRef led to a 404 error. [#8563](https://github.com/JabRef/jabref/issues/8563)
- We fixed an issue where not all found unlinked local files were imported correctly due to some race condition. [#8444](https://github.com/JabRef/jabref/issues/8444)
- We fixed an issue where Merge entries dialog exceeds screen boundaries.
- We fixed the page ranges checker to detect article numbers (used at [Check Integrity](https://docs.jabref.org/finding-sorting-and-cleaning-entries/checkintegrity)).
- We fixed an issue where the app lags when selecting an entry after a fresh start. [#8446](https://github.com/JabRef/jabref/issues/8446)
- We fixed an issue where no citationkey was generated on import, pasting a doi or an entry on the main table. [8406](https://github.com/JabRef/jabref/issues/8406), [koppor#553](https://github.com/koppor/jabref/issues/553)
- We fixed an issue where accent search does not perform consistently. [#6815](https://github.com/JabRef/jabref/issues/6815)
Expand Down
83 changes: 78 additions & 5 deletions src/main/java/org/jabref/logic/citationstyle/CSLAdapter.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,22 @@
import java.util.Arrays;
import java.util.Collection;
import java.util.List;
import java.util.Locale;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;

import org.jabref.logic.formatter.bibtexfields.RemoveNewlinesFormatter;
import org.jabref.logic.integrity.PagesChecker;
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.model.database.BibDatabaseMode;
import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.BibEntryType;
import org.jabref.model.entry.BibEntryTypesManager;
import org.jabref.model.entry.Month;
import org.jabref.model.entry.field.Field;
import org.jabref.model.entry.field.StandardField;
import org.jabref.model.entry.types.StandardEntryType;
import org.jabref.model.strings.LatexToUnicodeAdapter;

import de.undercouch.citeproc.CSL;
Expand Down Expand Up @@ -73,14 +77,15 @@ public synchronized List<String> makeBibliography(List<BibEntry> bibEntries, Str
* @throws IOException An error occurred in the underlying JavaScript framework
*/
private void initialize(String newStyle, CitationStyleOutputFormat newFormat) throws IOException {
if ((cslInstance == null) || !Objects.equals(newStyle, style)) {
final boolean newCslInstanceNeedsToBeCreated = (cslInstance == null) || !Objects.equals(newStyle, style);
if (newCslInstanceNeedsToBeCreated) {
// lang and forceLang are set to the default values of other CSL constructors
cslInstance = new CSL(dataProvider, new JabRefLocaleProvider(),
new DefaultAbbreviationProvider(), newStyle, "en-US");
style = newStyle;
}

if (!Objects.equals(newFormat, format)) {
if (newCslInstanceNeedsToBeCreated || (!Objects.equals(newFormat, format))) {
cslInstance.setOutputFormat(newFormat.getFormat());
format = newFormat;
}
Expand All @@ -95,11 +100,15 @@ private static class JabRefItemDataProvider implements ItemDataProvider {
private final List<BibEntry> data = new ArrayList<>();
private BibDatabaseContext bibDatabaseContext;
private BibEntryTypesManager entryTypesManager;
private PagesChecker pagesChecker;

/**
* Converts the {@link BibEntry} into {@link CSLItemData}.
*/
private static CSLItemData bibEntryToCSLItemData(BibEntry bibEntry, BibDatabaseContext bibDatabaseContext, BibEntryTypesManager entryTypesManager) {
private CSLItemData bibEntryToCSLItemData(BibEntry originalBibEntry, BibDatabaseContext bibDatabaseContext, BibEntryTypesManager entryTypesManager) {
// We need to make a deep copy, because we modify the entry according to the logic presented at
// https://github.com/JabRef/jabref/issues/8372#issuecomment-1014941935
BibEntry bibEntry = (BibEntry) originalBibEntry.clone();
String citeKey = bibEntry.getCitationKey().orElse("");
BibTeXEntry bibTeXEntry = new BibTeXEntry(new Key(bibEntry.getType().getName()), new Key(citeKey));

Expand All @@ -108,8 +117,66 @@ private static CSLItemData bibEntryToCSLItemData(BibEntry bibEntry, BibDatabaseC

Optional<BibEntryType> entryType = entryTypesManager.enrich(bibEntry.getType(), bibDatabaseContext.getMode());

Set<Field> fields = entryType.map(BibEntryType::getAllFields).orElse(bibEntry.getFields());
if (bibEntry.getType().equals(StandardEntryType.Article)) {
// Patch bibEntry to contain the right BibTeX (not BibLaTeX) fields
// Note that we do not need to convert from "pages" to "page", because CiteProc already handles it
// See BibTeXConverter
if (bibDatabaseContext.isBiblatexMode()) {
// Map "number" to CSL "issue", unless no number exists
Optional<String> numberField = bibEntry.getField(StandardField.NUMBER);
numberField.ifPresent(number -> {
bibEntry.setField(StandardField.ISSUE, number);
bibEntry.clearField(StandardField.NUMBER);
}
// TODO: Map eid to CSL number, once APA recognizes a "number" field
// www.zotero.org/styles/apa-6th-edition
// https://docs.citationstyles.org/en/stable/specification.html#number-variables
// https://apastyle.apa.org/style-grammar-guidelines/references/examples/journal-article-references#2
// e.g. like this:
//
// bibEntry.getField(StandardField.EID).ifPresent(eid -> {
// if (!bibEntry.hasField(StandardField.NUMBER)) {
// bibEntry.setField(StandardField.NUMBER, eid);
// bibEntry.clearField(StandardField.EID);
// }
// else {
// if (!bibEntry.hasField(StandardField.PAGES)) {
// bibEntry.setField(StandardField.PAGES, eid);
// bibEntry.clearField(StandardField.EID);
// }
// }});
//
// TODO: Remove mapping of eid to pages once eid is mapped to number.
);
bibEntry.getField(StandardField.EID).ifPresent(eid -> {
if (!bibEntry.hasField(StandardField.PAGES)) {
bibEntry.setField(StandardField.PAGES, eid);
bibEntry.clearField(StandardField.EID);
}}
koppor marked this conversation as resolved.
Show resolved Hide resolved
);
} else {
// BibTeX mode
bibEntry.getField(StandardField.NUMBER).ifPresent(number -> {
bibEntry.setField(StandardField.ISSUE, number);
bibEntry.clearField(StandardField.NUMBER);
});
bibEntry.getField(StandardField.PAGES).ifPresent(pages -> {
ThiloteE marked this conversation as resolved.
Show resolved Hide resolved
if (pages.toLowerCase(Locale.ROOT).startsWith("article ")) {
pages = pages.substring("Article ".length());
bibEntry.setField(StandardField.NUMBER, pages);
}
});
bibEntry.getField(StandardField.EID).ifPresent(eid -> {
if (!bibEntry.hasField(StandardField.PAGES)) {
ThiloteE marked this conversation as resolved.
Show resolved Hide resolved
bibEntry.setField(StandardField.PAGES, eid);
bibEntry.clearField(StandardField.EID);
}
});
}
}

Set<Field> fields = entryType.map(BibEntryType::getAllFields).orElse(bibEntry.getFields());
fields.addAll(bibEntry.getFields());
for (Field key : fields) {
bibEntry.getResolvedFieldOrAlias(key, bibDatabaseContext.getDatabase())
.map(removeNewlinesFormatter::format)
Expand All @@ -130,13 +197,19 @@ public void setData(List<BibEntry> data, BibDatabaseContext bibDatabaseContext,
this.data.addAll(data);
this.bibDatabaseContext = bibDatabaseContext;
this.entryTypesManager = entryTypesManager;

// Quick solution to always use BibLaTeX mode at the checker to allow pages ranges with single dash, too
// Example: pages = {1-2}
BibDatabaseContext ctx = new BibDatabaseContext();
ctx.setMode(BibDatabaseMode.BIBLATEX);
this.pagesChecker = new PagesChecker(ctx);
}

@Override
public CSLItemData retrieveItem(String id) {
return data.stream()
.filter(entry -> entry.getCitationKey().orElse("").equals(id))
.map(entry -> JabRefItemDataProvider.bibEntryToCSLItemData(entry, bibDatabaseContext, entryTypesManager))
.map(entry -> bibEntryToCSLItemData(entry, bibDatabaseContext, entryTypesManager))
.findFirst().orElse(null);
}

Expand Down
27 changes: 12 additions & 15 deletions src/main/java/org/jabref/logic/integrity/PagesChecker.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.jabref.logic.integrity;

import java.util.Arrays;
import java.util.Optional;
import java.util.function.Predicate;
import java.util.regex.Pattern;
Expand All @@ -12,27 +13,22 @@ public class PagesChecker implements ValueChecker {

private static final String PAGES_EXP_BIBTEX = ""
+ "\\A" // begin String
+ "[A-Za-z]?\\d*" // optional prefix and number
+ "("
+ "[A-Za-z]?\\d*" // optional prefix and number
+ "("
+ "\\+|-{2}" // separator
+ "[A-Za-z]?\\d*" // optional prefix and number
+ "(\\+|-{2}|\u2013)" // separator, must contain exactly two dashes
+ "[A-Za-z]?\\d*" // optional prefix and number
+ ")?"
+ ",?" // page range separation
+ ")*"
+ "\\z"; // end String

// See https://packages.oth-regensburg.de/ctan/macros/latex/contrib/biblatex/doc/biblatex.pdf#subsubsection.3.15.3 for valid content
private static final String PAGES_EXP_BIBLATEX = ""
+ "\\A" // begin String
+ "("
+ "\\A" // begin String
+ "[A-Za-z]?\\d*" // optional prefix and number
+ "("
+ "\\+|-{1,2}|\u2013" // separator
+ "[A-Za-z]?\\d*" // optional prefix and number
+ "(\\+|-{1,2}|\u2013)" // separator
+ "[A-Za-z]?\\d*" // optional prefix and number
+ ")?"
+ ",?" // page range separation
+ ")*"
+ "\\z"; // end String
+ "\\z"; // end String

private final Predicate<String> isValidPageNumber;

Expand All @@ -59,10 +55,11 @@ public Optional<String> checkValue(String value) {
return Optional.empty();
}

if (!isValidPageNumber.test(value.trim())) {
if (Arrays.stream(value.split(","))
.map(String::trim)
.anyMatch(pageRange -> !isValidPageNumber.test(pageRange))) {
return Optional.of(Localization.lang("should contain a valid page number range"));
}

return Optional.empty();
}
}
Loading