Preserve user comments in bib file (#1471)

* Add failing test with preceding comment * Add failing parser test * Improve test naming * Reuse Globals.ENCODING_PREFIX in test * Check explicitly for encoding line and purge it * Add changelog entry * Write out user comments also for modified entries * Add test to check preservation of ENCODING_PREFIX inside an entry * Make BibEntryWriter more robust when dealing with faulty parsed serializations * Add test with user comment in file * Add test that changes and reformats an entry with a user comment * Add test with user comment before String * Preserve newlines also when used with bibtex strings * Add test for serialization of epilog * Fix string detection in test * In case of change, only remove trailing whitespaces between user comments and entry * Remove unused variable * Remove unused import * Reformat changelog entry * Move comment detection logic to BibtexString * Compute user comment in string only once * Move user comment computation to BibEntry * Add test for comment in the same line as BibEntry * Remove unnecessary epilog test * Remove redundant test bib file * Remove redundant asserts * Elevate registry actions * Delete stuff * Revert "Elevate registry actions" This reverts commit b4b288a. * Revert "Delete stuff" This reverts commit 67a4885. * Use optional in assert * Remove duplicate test * Remove unnecessary asserts * Remove unused import of Optional
JabRef · Jul 14, 2016 · aa42c16 · aa42c16
1 parent 4bda9c9
commit aa42c16
Show file tree

Hide file tree

Showing 12 changed files with 438 additions and 152 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -12,6 +12,7 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `#
 ## [Unreleased]
 
 ### Changed
+- [#1026](https://github.com/JabRef/jabref/issues/1026) JabRef does no longer delete user comments outside of BibTeX entries and strings
 
 ### Fixed
 

diff --git a/src/main/java/net/sf/jabref/exporter/BibDatabaseWriter.java b/src/main/java/net/sf/jabref/exporter/BibDatabaseWriter.java
@@ -317,6 +317,8 @@ private void writeString(Writer fw, BibtexString bs, Map<String, BibtexString> r
             return;
         }
 
+        writeUserCommentsForString(bs, fw);
+
         if(isFirstStringInType) {
             fw.write(Globals.NEWLINE);
         }
@@ -354,6 +356,14 @@ private void writeString(Writer fw, BibtexString bs, Map<String, BibtexString> r
         fw.write("}" + Globals.NEWLINE);
     }
 
+    private void writeUserCommentsForString(BibtexString string, Writer out) throws IOException {
+        String userComments = string.getUserComments();
+
+        if(!userComments.isEmpty()) {
+            out.write(userComments + Globals.NEWLINE);
+        }
+    }
+
     /**
      * Write all strings in alphabetical order, modified to produce a safe (for
      * BibTeX) order of the strings if they reference each other.

diff --git a/src/main/java/net/sf/jabref/importer/fileformat/BibtexParser.java b/src/main/java/net/sf/jabref/importer/fileformat/BibtexParser.java
@@ -29,6 +29,7 @@
 import java.util.Objects;
 import java.util.Optional;
 
+import net.sf.jabref.Globals;
 import net.sf.jabref.MetaData;
 import net.sf.jabref.importer.ParserResult;
 import net.sf.jabref.logic.bibtex.FieldContentParser;
@@ -323,29 +324,23 @@ private String dumpTextReadSoFarToString() {
         // if there is no entry found, simply return the content (necessary to parse text remaining after the last entry)
         if (indexOfAt == -1) {
             return purgeEOFCharacters(result);
-        } else {
-
-            //skip all text except newlines and whitespaces before first @. This is necessary to remove the file header
-            int runningIndex = indexOfAt - 1;
-            while (runningIndex >= 0) {
-                if (!Character.isWhitespace(result.charAt(runningIndex))) {
+        } else if(result.contains(Globals.ENCODING_PREFIX)) {
+            // purge the encoding line if it exists
+            int runningIndex = result.indexOf(Globals.ENCODING_PREFIX);
+            while(runningIndex < indexOfAt) {
+                if(result.charAt(runningIndex) == '\n') {
+                    break;
+                } else if(result.charAt(runningIndex) == '\r') {
+                    if(result.charAt(runningIndex + 1) == '\n') {
+                        runningIndex++;
+                    }
                     break;
                 }
-                runningIndex--;
-            }
-
-            if(runningIndex > -1) {
-                // We have to ignore some text at the beginning
-                // so we view the first line break as the end of the previous text and don't store it
-                if(result.charAt(runningIndex + 1) == '\r') {
-                    runningIndex++;
-                }
-                if(result.charAt(runningIndex + 1) == '\n') {
-                    runningIndex++;
-                }
+                runningIndex++;
             }
-
             return result.substring(runningIndex + 1);
+        } else {
+            return result;
         }
     }
 

diff --git a/src/main/java/net/sf/jabref/logic/bibtex/BibEntryWriter.java b/src/main/java/net/sf/jabref/logic/bibtex/BibEntryWriter.java
@@ -37,22 +37,32 @@ public void write(BibEntry entry, Writer out, BibDatabaseMode bibDatabaseMode) t
     /**
      * Writes the given BibEntry using the given writer
      *
-     * @param entry The entry to write
-     * @param out The writer to use
+     * @param entry           The entry to write
+     * @param out             The writer to use
      * @param bibDatabaseMode The database mode (bibtex or biblatex)
-     * @param reformat Should the entry be in any case, even if no change occurred?
+     * @param reformat        Should the entry be in any case, even if no change occurred?
      */
     public void write(BibEntry entry, Writer out, BibDatabaseMode bibDatabaseMode, Boolean reformat) throws IOException {
         // if the entry has not been modified, write it as it was
         if (!reformat && !entry.hasChanged()) {
             out.write(entry.getParsedSerialization());
             return;
         }
+
+        writeUserComments(entry, out);
         out.write(Globals.NEWLINE);
         writeRequiredFieldsFirstRemainingFieldsSecond(entry, out, bibDatabaseMode);
         out.write(Globals.NEWLINE);
     }
 
+    private void writeUserComments(BibEntry entry, Writer out) throws IOException {
+        String userComments = entry.getUserComments();
+
+        if(!userComments.isEmpty()) {
+            out.write(userComments + Globals.NEWLINE);
+        }
+    }
+
     public void writeWithoutPrependedNewlines(BibEntry entry, Writer out, BibDatabaseMode bibDatabaseMode) throws IOException {
         // if the entry has not been modified, write it as it was
         if (!entry.hasChanged()) {
@@ -71,7 +81,7 @@ public void writeWithoutPrependedNewlines(BibEntry entry, Writer out, BibDatabas
      * @throws IOException
      */
     private void writeRequiredFieldsFirstRemainingFieldsSecond(BibEntry entry, Writer out,
-            BibDatabaseMode bibDatabaseMode) throws IOException {
+                                                               BibDatabaseMode bibDatabaseMode) throws IOException {
         // Write header with type and bibtex-key.
         TypedBibEntry typedEntry = new TypedBibEntry(entry, Optional.empty(), bibDatabaseMode);
         out.write('@' + typedEntry.getTypeForDisplay() + '{');
@@ -127,9 +137,9 @@ private void writeKeyField(BibEntry entry, Writer out) throws IOException {
     /**
      * Write a single field, if it has any content.
      *
-     * @param entry             the entry to write
-     * @param out               the target of the write
-     * @param name              The field name
+     * @param entry the entry to write
+     * @param out   the target of the write
+     * @param name  The field name
      * @throws IOException In case of an IO error
      */
     private void writeField(BibEntry entry, Writer out, String name, int indentation) throws IOException {

diff --git a/src/main/java/net/sf/jabref/model/entry/BibEntry.java b/src/main/java/net/sf/jabref/model/entry/BibEntry.java
@@ -629,6 +629,31 @@ public BibEntry withField(String field, String value) {
         return this;
     }
 
+    /*
+    * Returns user comments (arbitrary text before the entry), if they exist. If not, returns the empty String
+     */
+    public String getUserComments() {
+
+        if (parsedSerialization != null) {
+
+            try {
+                // get the text before the entry
+                String prolog = parsedSerialization.substring(0, parsedSerialization.indexOf('@'));
+
+                // delete trailing whitespaces (between entry and text)
+                prolog = prolog.replaceFirst("\\s+$", "");
+
+                // if there is any non whitespace text, write it
+                if (prolog.length() > 0) {
+                    return prolog;
+                }
+            } catch (StringIndexOutOfBoundsException ignore) {
+                // if this occurs a broken parsed serialization has been set, so just do nothing
+            }
+        }
+        return "";
+    }
+
     public Set<String> getFieldAsWords(String field) {
         String fieldName = toLowerCase(field);
         Set<String> storedList = fieldsAsWords.get(fieldName);

diff --git a/src/main/java/net/sf/jabref/model/entry/BibtexString.java b/src/main/java/net/sf/jabref/model/entry/BibtexString.java
@@ -152,4 +152,28 @@ public String getParsedSerialization() {
     public boolean hasChanged(){
         return hasChanged;
     }
+
+    /*
+    * Returns user comments (arbitrary text before the string) if there are any. If not returns the empty string
+     */
+    public String getUserComments() {
+        if(parsedSerialization != null) {
+
+            try {
+                // get the text before the string
+                String prolog = parsedSerialization.substring(0, parsedSerialization.indexOf('@'));
+
+                // delete trailing whitespaces (between string and text)
+                prolog = prolog.replaceFirst("\\s+$", "");
+                // if there is any non whitespace text, write it with proper line separation
+                if (prolog.length() > 0) {
+                    return prolog;
+                }
+            } catch(StringIndexOutOfBoundsException ignore) {
+                // if this occurs a broken parsed serialization has been set, so just do nothing
+            }
+        }
+
+        return "";
+    }
 }
diff --git a/src/test/java/net/sf/jabref/exporter/BibDatabaseWriterTest.java b/src/test/java/net/sf/jabref/exporter/BibDatabaseWriterTest.java
@@ -291,6 +291,66 @@ public void roundtrip() throws IOException {
         }
     }
 
+    @Test
+    public void roundtripWithUserComment() throws IOException {
+        Path testBibtexFile = Paths.get("src/test/resources/testbib/bibWithUserComments.bib");
+        Charset encoding = StandardCharsets.UTF_8;
+        ParserResult result = BibtexParser.parse(ImportFormat.getReader(testBibtexFile, encoding));
+
+        SavePreferences preferences = new SavePreferences().withEncoding(encoding).withSaveInOriginalOrder(true);
+        BibDatabaseContext context = new BibDatabaseContext(result.getDatabase(), result.getMetaData(),
+                new Defaults(BibDatabaseMode.BIBTEX));
+
+        databaseWriter.writePartOfDatabase(stringWriter, context, result.getDatabase().getEntries(), preferences);
+        try (Scanner scanner = new Scanner(testBibtexFile,encoding.name())) {
+            assertEquals(scanner.useDelimiter("\\A").next(), stringWriter.toString());
+        }
+    }
+
+    @Test
+    public void roundtripWithUserCommentAndEntryChange() throws IOException {
+        Path testBibtexFile = Paths.get("src/test/resources/testbib/bibWithUserComments.bib");
+        Charset encoding = StandardCharsets.UTF_8;
+        ParserResult result = BibtexParser.parse(ImportFormat.getReader(testBibtexFile, encoding));
+
+        BibEntry entry = result.getDatabase().getEntryByKey("1137631").get();
+        entry.setField("author", "Mr. Author");
+
+        SavePreferences preferences = new SavePreferences().withEncoding(encoding).withSaveInOriginalOrder(true);
+        BibDatabaseContext context = new BibDatabaseContext(result.getDatabase(), result.getMetaData(),
+                new Defaults(BibDatabaseMode.BIBTEX));
+
+        databaseWriter.writePartOfDatabase(stringWriter, context, result.getDatabase().getEntries(), preferences);
+
+        try (Scanner scanner = new Scanner(Paths.get("src/test/resources/testbib/bibWithUserCommentAndEntryChange.bib"),encoding.name())) {
+            assertEquals(scanner.useDelimiter("\\A").next(), stringWriter.toString());
+        }
+    }
+
+    @Test
+    public void roundtripWithUserCommentBeforeStringAndChange() throws IOException {
+        Path testBibtexFile = Paths.get("src/test/resources/testbib/complex.bib");
+        Charset encoding = StandardCharsets.UTF_8;
+        ParserResult result = BibtexParser.parse(ImportFormat.getReader(testBibtexFile, encoding));
+
+        BibtexString string = result.getDatabase().getStringValues().iterator().next();
+        if(string.getContent().isEmpty()) {
+            // do nothing
+        } else {
+            string.setContent("my first string");
+        }
+
+        SavePreferences preferences = new SavePreferences().withEncoding(encoding).withSaveInOriginalOrder(true);
+        BibDatabaseContext context = new BibDatabaseContext(result.getDatabase(), result.getMetaData(),
+                new Defaults(BibDatabaseMode.BIBTEX));
+
+        databaseWriter.writePartOfDatabase(stringWriter, context, result.getDatabase().getEntries(), preferences);
+
+        try (Scanner scanner = new Scanner(testBibtexFile,encoding.name())) {
+            assertEquals(scanner.useDelimiter("\\A").next(), stringWriter.toString());
+        }
+    }
+
     @Test
     public void writeSavedSerializationOfEntryIfUnchanged() throws IOException {
         BibEntry entry = new BibEntry();
@@ -543,4 +603,5 @@ public void writeEntriesInOriginalOrderWhenNoSaveOrderConfigIsSetInMetadata() th
                         + Globals.NEWLINE
                 , stringWriter.toString());
     }
+
 }