Fix CSL rendering in case of article

CHANGELOG.md

@@ -25,12 +25,15 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
 - We changed the button label from "Return to JabRef" to "Return to library" to better indicate the purpose of the action.
 - We reworked the External Changes Resolver dialog. [#9021](https://github.com/JabRef/jabref/pull/9021)
 
+
 ### Fixed
 
+- We fixed the display of issue, number, eid and pages fields in the entry preview. [#8607](https://github.com/JabRef/jabref/pull/8607), [#8372](https://github.com/JabRef/jabref/issues/8372), [Koppor#514](https://github.com/koppor/jabref/issues/514), [forum#2390](https://discourse.jabref.org/t/unable-to-edit-my-bibtex-file-that-i-used-before-vers-5-1/2390), [forum#3462](https://discourse.jabref.org/t/jabref-5-6-need-help-with-export-from-jabref-to-microsoft-word-entry-preview-of-apa-7-not-rendering-correctly/3462)
+- We fixed the page ranges checker to detect article numbers in the pages field (used at [Check Integrity](https://docs.jabref.org/finding-sorting-and-cleaning-entries/checkintegrity)). [#8607](https://github.com/JabRef/jabref/pull/8607)
 - The [HtmlToLaTeXFormatter](https://docs.jabref.org/finding-sorting-and-cleaning-entries/saveactions#html-to-latex) keeps single `<` characters.
 - We fixed a performance regression when opening large libraries [#9041](https://github.com/JabRef/jabref/issues/9041)
 - We fixed a bug where spaces are trimmed when highlighting differences in the Entries merge dialog. [koppor#371](https://github.com/koppor/jabref/issues/371)
-- We fixed several bugs regarding the manual and the autosave of library files that sometimes lead to exceptions or data loss. [#8448](https://github.com/JabRef/jabref/issues/8484), [#8746](https://github.com/JabRef/jabref/issues/8746), [#6684](https://github.com/JabRef/jabref/issues/6684), [#6644](https://github.com/JabRef/jabref/issues/6644), [#6102](https://github.com/JabRef/jabref/issues/6102), [#6002](https://github.com/JabRef/jabref/issues/6000)
+- We fixed several bugs regarding the manual and the autosave of library files that sometimes lead to exceptions or data loss. [#9067](https://github.com/JabRef/jabref/pull/9067), [#8448](https://github.com/JabRef/jabref/issues/8484), [#8746](https://github.com/JabRef/jabref/issues/8746), [#6684](https://github.com/JabRef/jabref/issues/6684), [#6644](https://github.com/JabRef/jabref/issues/6644), [#6102](https://github.com/JabRef/jabref/issues/6102), [#6002](https://github.com/JabRef/jabref/issues/6000)
 - We fixed an issue where applied save actions on saving the library file would lead to the dialog "The libary has been modified by another program" popping up [#4877](https://github.com/JabRef/jabref/issues/4877)
 - We fixed an issue where JabRef would not exit when a connection to a LibreOffice document was established previously and the document is still open [#9075](https://github.com/JabRef/jabref/issues/9075)
 

src/main/java/org/jabref/logic/citationstyle/CSLAdapter.java

@@ -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;
@@ -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;
         }
@@ -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));
 
@@ -108,8 +117,55 @@ 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: fix tests in CitationStyleGeneratorTest.java using APA style, once APA recognizes a "number" field (and thereby will be able to render the "eid" field)
+                 * tracked at https://github.com/citation-style-language/styles/issues/5827
+                 *
+                 * Further info:
+                 * 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
+                 */
+                  bibEntry.getField(StandardField.EID).ifPresent(eid -> {
+                   if (!bibEntry.hasField(StandardField.NUMBER)) {
+                       bibEntry.setField(StandardField.NUMBER, eid);
+                       bibEntry.clearField(StandardField.EID);
+                        }
+                    });
+                } else {
+                    // BibTeX mode
+                    bibEntry.getField(StandardField.NUMBER).ifPresent(number -> {
+                        bibEntry.setField(StandardField.ISSUE, number);
+                        bibEntry.clearField(StandardField.NUMBER);
+                    });
+                    bibEntry.getField(StandardField.PAGES).ifPresent(pages -> {
+                        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)) {
+                            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)
@@ -130,13 +186,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);
         }
 

src/main/java/org/jabref/logic/integrity/PagesChecker.java

@@ -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;
@@ -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;
 
@@ -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();
     }
 }