Fix #3805: dash correctly terminates commands in layout files

JabRef · Jun 17, 2018 · 920505a · 920505a
1 parent b4b632b
commit 920505a
Show file tree

Hide file tree

Showing 6 changed files with 76 additions and 77 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -31,6 +31,7 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `#
 - We fixed an issue where in rare cases entries where overlayed in the main table. https://github.com/JabRef/jabref/issues/3281
 - We fixed an issue where selecting a group messed up the focus of the main table / entry editor. https://github.com/JabRef/jabref/issues/3367
 - We fixed an issue where composite author names were sorted incorrectly. https://github.com/JabRef/jabref/issues/2828
+- We fixed an issue where commands followed by `-` didn't worked. [#3805](https://github.com/JabRef/jabref/issues/3805)
 - We fixed an issue where the custom file column were sorted incorrectly. https://github.com/JabRef/jabref/issues/3119
 - We fixed an issues where the entry losses focus when a field is edited and at the same time used for sorting. https://github.com/JabRef/jabref/issues/3373
 - We fixed an issue where the menu on Mac OS was not displayed in the usual Mac-specific way. https://github.com/JabRef/jabref/issues/3146

diff --git a/src/main/java/org/jabref/logic/layout/Layout.java b/src/main/java/org/jabref/logic/layout/Layout.java
@@ -33,7 +33,7 @@ public Layout(List<StringInt> parsedEntries, LayoutFormatterPreferences prefs) {
         for (StringInt parsedEntry : parsedEntries) {
             switch (parsedEntry.i) {
             case LayoutHelper.IS_LAYOUT_TEXT:
-            case LayoutHelper.IS_SIMPLE_FIELD:
+                case LayoutHelper.IS_SIMPLE_COMMAND:
             case LayoutHelper.IS_OPTION_FIELD:
                 // Do nothing
                 break;

diff --git a/src/main/java/org/jabref/logic/layout/LayoutEntry.java b/src/main/java/org/jabref/logic/layout/LayoutEntry.java
@@ -109,7 +109,7 @@ public LayoutEntry(StringInt si, LayoutFormatterPreferences prefs) {
         case LayoutHelper.IS_LAYOUT_TEXT:
             text = si.s;
             break;
-        case LayoutHelper.IS_SIMPLE_FIELD:
+            case LayoutHelper.IS_SIMPLE_COMMAND:
             text = si.s.trim();
             break;
         case LayoutHelper.IS_OPTION_FIELD:
@@ -156,7 +156,7 @@ public LayoutEntry(List<StringInt> parsedEntries, int layoutType, LayoutFormatte
                 }
                 break;
             case LayoutHelper.IS_LAYOUT_TEXT:
-            case LayoutHelper.IS_SIMPLE_FIELD:
+                case LayoutHelper.IS_SIMPLE_COMMAND:
             case LayoutHelper.IS_OPTION_FIELD:
             default:
                 // Do nothing
@@ -185,7 +185,7 @@ public String doLayout(BibEntry bibtex, BibDatabase database) {
         switch (type) {
         case LayoutHelper.IS_LAYOUT_TEXT:
             return text;
-        case LayoutHelper.IS_SIMPLE_FIELD:
+            case LayoutHelper.IS_SIMPLE_COMMAND:
             String value = bibtex.getResolvedFieldOrAlias(text, database).orElse("");
 
             // If a post formatter has been set, call it:
@@ -331,7 +331,7 @@ public String doLayout(BibDatabaseContext databaseContext, Charset encoding) {
         case LayoutHelper.IS_LAYOUT_TEXT:
             return text;
 
-        case LayoutHelper.IS_SIMPLE_FIELD:
+            case LayoutHelper.IS_SIMPLE_COMMAND:
             throw new UnsupportedOperationException("bibtex entry fields not allowed in begin or end layout");
 
         case LayoutHelper.IS_FIELD_START:

diff --git a/src/main/java/org/jabref/logic/layout/LayoutHelper.java b/src/main/java/org/jabref/logic/layout/LayoutHelper.java
@@ -20,7 +20,7 @@
 public class LayoutHelper {
 
     public static final int IS_LAYOUT_TEXT = 1;
-    public static final int IS_SIMPLE_FIELD = 2;
+    public static final int IS_SIMPLE_COMMAND = 2;
     public static final int IS_FIELD_START = 3;
     public static final int IS_FIELD_END = 4;
     public static final int IS_OPTION_FIELD = 5;
@@ -47,7 +47,7 @@ public Layout getLayoutFromText() throws IOException {
         parse();
 
         for (StringInt parsedEntry : parsedEntries) {
-            if ((parsedEntry.i == LayoutHelper.IS_SIMPLE_FIELD) || (parsedEntry.i == LayoutHelper.IS_FIELD_START)
+            if ((parsedEntry.i == LayoutHelper.IS_SIMPLE_COMMAND) || (parsedEntry.i == LayoutHelper.IS_FIELD_START)
                     || (parsedEntry.i == LayoutHelper.IS_FIELD_END) || (parsedEntry.i == LayoutHelper.IS_GROUP_START)
                     || (parsedEntry.i == LayoutHelper.IS_GROUP_END)) {
                 parsedEntry.s = parsedEntry.s.trim().toLowerCase(Locale.ROOT);
@@ -209,10 +209,7 @@ private void parse() throws IOException, StringIndexOutOfBoundsException {
             if (c == -1) {
                 endOfFile = true;
 
-                /*
-                 * CO 2006-11-11: Added check for null, otherwise a Layout that
-                 * finishes with a curly brace throws a NPE
-                 */
+                // Check for null, otherwise a Layout that finishes with a curly brace throws a NPE
                 if (buffer != null) {
                     parsedEntries.add(new StringInt(buffer.toString(), LayoutHelper.IS_LAYOUT_TEXT));
                 }
@@ -258,7 +255,7 @@ private void parseField() throws IOException {
                 endOfFile = true;
             }
 
-            if (!Character.isLetter((char) c) && (c != '_') && (c != '-')) {
+            if (!Character.isLetter((char) c) && (c != '_')) {
                 unread(c);
 
                 name = buffer == null ? "" : buffer.toString();
@@ -323,8 +320,8 @@ private void parseField() throws IOException {
                     return;
                 }
 
-                // for all other cases
-                parsedEntries.add(new StringInt(name, LayoutHelper.IS_SIMPLE_FIELD));
+                // for all other cases -> simple command
+                parsedEntries.add(new StringInt(name, LayoutHelper.IS_SIMPLE_COMMAND));
 
                 return;
             } else {

diff --git a/src/main/java/org/jabref/model/entry/BibEntry.java b/src/main/java/org/jabref/model/entry/BibEntry.java
@@ -76,7 +76,6 @@ public class BibEntry implements Cloneable {
     /**
      * Constructs a new BibEntry. The internal ID is set to IdGenerator.next()
      */
-
     public BibEntry() {
         this(IdGenerator.next(), DEFAULT_TYPE);
     }
@@ -85,7 +84,9 @@ public BibEntry() {
      * Constructs a new BibEntry with the given type
      *
      * @param type The type to set. May be null or empty. In that case, DEFAULT_TYPE is used.
+     * @deprecated use {{@link #BibEntry(EntryType)}} instead
      */
+    @Deprecated
     public BibEntry(String type) {
         this(IdGenerator.next(), type);
     }
@@ -104,6 +105,13 @@ private BibEntry(String id, String type) {
         this.sharedBibEntryData = new SharedBibEntryData();
     }
 
+    /**
+     * Constructs a new BibEntry. The internal ID is set to IdGenerator.next()
+     */
+    public BibEntry(EntryType type) {
+        this(type.getName());
+    }
+
     public Optional<FieldChange> setMonth(Month parsedMonth) {
         return setField(FieldName.MONTH, parsedMonth.getJabRefFormat());
     }

diff --git a/src/test/java/org/jabref/logic/layout/LayoutTest.java b/src/test/java/org/jabref/logic/layout/LayoutTest.java
@@ -3,15 +3,12 @@
 import java.io.File;
 import java.io.IOException;
 import java.io.StringReader;
-import java.util.Collection;
 import java.util.Collections;
 
-import org.jabref.logic.importer.ImportFormatPreferences;
-import org.jabref.logic.importer.ParserResult;
-import org.jabref.logic.importer.fileformat.BibtexParser;
 import org.jabref.logic.layout.format.FileLinkPreferences;
 import org.jabref.model.entry.BibEntry;
-import org.jabref.model.util.DummyFileUpdateMonitor;
+import org.jabref.model.entry.BibtexEntryTypes;
+import org.jabref.model.entry.LinkedFile;
 
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -21,93 +18,79 @@
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
-public class LayoutTest {
+class LayoutTest {
 
-    private static ImportFormatPreferences importFormatPreferences;
     private LayoutFormatterPreferences layoutFormatterPreferences;
 
-    /**
-     * Initialize Preferences.
-     */
     @BeforeEach
-    public void setUp() {
+    void setUp() {
         layoutFormatterPreferences = mock(LayoutFormatterPreferences.class, Answers.RETURNS_DEEP_STUBS);
-        importFormatPreferences = mock(ImportFormatPreferences.class, Answers.RETURNS_DEEP_STUBS);
     }
 
-    /**
-     * Return Test data.
-     */
-    public String t1BibtexString() {
-        return "@article{canh05,\n" + "  author = {This\nis\na\ntext},\n"
-                + "  title = {Effective work practices for floss development: A model and propositions},\n"
-                + "  booktitle = {Hawaii International Conference On System Sciences (HICSS)},\n" + "  year = {2005},\n"
-                + "  owner = {oezbek},\n" + "  timestamp = {2006.05.29},\n"
-                + "  url = {http://james.howison.name/publications.html},\n" + "  abstract = {\\~{n} \\~n "
-                + "\\'i \\i \\i}\n" + "}\n";
-    }
+    private String layout(String layout, BibEntry entry) throws IOException {
+        StringReader layoutStringReader = new StringReader(layout.replace("__NEWLINE__", "\n"));
 
-    public static BibEntry bibtexString2BibtexEntry(String s) throws IOException {
-        ParserResult result = new BibtexParser(importFormatPreferences, new DummyFileUpdateMonitor()).parse(new StringReader(s));
-        Collection<BibEntry> c = result.getDatabase().getEntries();
-        assertEquals(1, c.size());
-        return c.iterator().next();
+        return new LayoutHelper(layoutStringReader, layoutFormatterPreferences)
+                .getLayoutFromText()
+                .doLayout(entry, null);
     }
 
-    public String layout(String layoutFile, String entry) throws IOException {
-        BibEntry be = LayoutTest.bibtexString2BibtexEntry(entry);
-        StringReader sr = new StringReader(layoutFile.replace("__NEWLINE__", "\n"));
-        Layout layout = new LayoutHelper(sr, layoutFormatterPreferences).getLayoutFromText();
+    @Test
+    void entryTypeForUnknown() throws IOException {
+        BibEntry entry = new BibEntry("unknown").withField("author", "test");
 
-        return layout.doLayout(be, null);
+        assertEquals("Unknown", layout("\\bibtextype", entry));
     }
 
     @Test
-    public void testLayoutBibtextype() throws IOException {
-        assertEquals("Unknown", layout("\\bibtextype", "@unknown{bla, author={This\nis\na\ntext}}"));
-        assertEquals("Article", layout("\\bibtextype", "@article{bla, author={This\nis\na\ntext}}"));
-        assertEquals("Misc", layout("\\bibtextype", "@misc{bla, author={This\nis\na\ntext}}"));
+    void entryTypeForArticle() throws IOException {
+        BibEntry entry = new BibEntry(BibtexEntryTypes.ARTICLE).withField("author", "test");
+
+        assertEquals("Article", layout("\\bibtextype", entry));
     }
 
     @Test
-    public void testHTMLChar() throws IOException {
-        String layoutText = layout("\\begin{author}\\format[HTMLChars]{\\author}\\end{author} ",
-                "@other{bla, author={This\nis\na\ntext}}");
+    void entryTypeForMisc() throws IOException {
+        BibEntry entry = new BibEntry(BibtexEntryTypes.MISC).withField("author", "test");
+
+        assertEquals("Misc", layout("\\bibtextype", entry));
+    }
 
-        assertEquals("This is a text ", layoutText);
+    @Test
+    void HTMLChar() throws IOException {
+        BibEntry entry = new BibEntry(BibtexEntryTypes.ARTICLE).withField("author", "This\nis\na\ntext");
 
-        layoutText = layout("\\begin{author}\\format[HTMLChars]{\\author}\\end{author}",
-                "@other{bla, author={This\nis\na\ntext}}");
+        String actual = layout("\\begin{author}\\format[HTMLChars]{\\author}\\end{author}", entry);
 
-        assertEquals("This is a text", layoutText);
+        assertEquals("This<br>is<br>a<br>text", actual);
     }
 
     @Test
-    public void testPluginLoading() throws IOException {
-        String layoutText = layout("\\begin{author}\\format[NameFormatter]{\\author}\\end{author}",
-                "@other{bla, author={Joe Doe and Jane, Moon}}");
+    void HTMLCharWithDoubleLineBreak() throws IOException {
+        BibEntry entry = new BibEntry(BibtexEntryTypes.ARTICLE).withField("author", "This\nis\na\n\ntext");
 
-        assertEquals("Joe Doe, Moon Jane", layoutText);
+        String layoutText = layout("\\begin{author}\\format[HTMLChars]{\\author}\\end{author} ", entry);
+
+        assertEquals("This<br>is<br>a<p>text ", layoutText);
     }
 
     @Test
-    public void testHTMLCharDoubleLineBreak() throws IOException {
-        String layoutText = layout("\\begin{author}\\format[HTMLChars]{\\author}\\end{author} ",
-                "@other{bla, author={This\nis\na\n\ntext}}");
+    void nameFormatter() throws IOException {
+        BibEntry entry = new BibEntry(BibtexEntryTypes.ARTICLE).withField("author", "Joe Doe and Jane, Moon");
 
-        assertEquals("This is a text ", layoutText);
+        String layoutText = layout("\\begin{author}\\format[NameFormatter]{\\author}\\end{author}", entry);
+
+        assertEquals("Joe Doe, Moon Jane", layoutText);
     }
 
-    /**
-     * [ 1495181 ] Dotless i and tilde not handled in preview
-     *
-     * @throws Exception
-     */
     @Test
-    public void testLayout() throws IOException {
+    void HTMLCharsWithDotlessIAndTiled() throws IOException {
+        BibEntry entry = new BibEntry(BibtexEntryTypes.ARTICLE)
+                .withField("abstract", "\\~{n} \\~n \\'i \\i \\i");
+
         String layoutText = layout(
                 "<font face=\"arial\">\\begin{abstract}<BR><BR><b>Abstract: </b> \\format[HTMLChars]{\\abstract}\\end{abstract}</font>",
-                t1BibtexString());
+                entry);
 
         assertEquals(
                 "<font face=\"arial\"><BR><BR><b>Abstract: </b> &ntilde; &ntilde; &iacute; &imath; &imath;</font>",
@@ -116,15 +99,25 @@ public void testLayout() throws IOException {
 
     @Test
     // Test for http://discourse.jabref.org/t/the-wrapfilelinks-formatter/172 (the example in the help files)
-    public void testWrapFileLinksLayout() throws IOException {
+    void wrapFileLinksExpandFile() throws IOException {
         when(layoutFormatterPreferences.getFileLinkPreferences()).thenReturn(
                 new FileLinkPreferences(Collections.emptyList(), Collections.singletonList("src/test/resources/pdfs/")));
+        BibEntry entry = new BibEntry(BibtexEntryTypes.ARTICLE);
+        entry.addFile(new LinkedFile("Test file", "encrypted.pdf", "PDF"));
 
-        String layoutText = layout("\\begin{file}\\format[WrapFileLinks(\\i. \\d (\\p))]{\\file}\\end{file}",
-                "@other{bla, file={Test file:encrypted.pdf:PDF}}");
+        String layoutText = layout("\\begin{file}\\format[WrapFileLinks(\\i. \\d (\\p))]{\\file}\\end{file}", entry);
 
         assertEquals(
                 "1. Test file (" + new File("src/test/resources/pdfs/encrypted.pdf").getCanonicalPath() + ")",
                 layoutText);
     }
+
+    @Test
+    void expandCommandIfTerminatedByMinus() throws IOException {
+        BibEntry entry = new BibEntry(BibtexEntryTypes.ARTICLE).withField("edition", "2");
+
+        String layoutText = layout("\\edition-th ed.-", entry);
+
+        assertEquals("2-th ed.-", layoutText);
+    }
 }