-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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 biblatex cleanup for date field #2339
Changes from 3 commits
ff5f1b5
ee6e40f
5cb09cd
65454d9
15f8c51
5610e22
efdf747
e09a43b
49d0e72
b7a3c8f
38c9488
07554e5
188838f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,14 +3,15 @@ | |
import java.util.ArrayList; | ||
import java.util.List; | ||
import java.util.Map; | ||
import java.util.Optional; | ||
|
||
import net.sf.jabref.model.FieldChange; | ||
import net.sf.jabref.model.cleanup.CleanupJob; | ||
import net.sf.jabref.model.entry.BibEntry; | ||
import net.sf.jabref.model.entry.EntryConverter; | ||
import net.sf.jabref.model.entry.FieldName; | ||
|
||
import org.apache.commons.lang3.StringUtils; | ||
|
||
/** | ||
* Converts the entry to BibLatex format. | ||
*/ | ||
|
@@ -25,32 +26,25 @@ public List<FieldChange> cleanup(BibEntry entry) { | |
entry.getField(oldFieldName).ifPresent(oldValue -> { | ||
if (!oldValue.isEmpty() && (!entry.getField(newFieldName).isPresent())) { | ||
// There is content in the old field and no value in the new, so just copy | ||
entry.setField(newFieldName, oldValue); | ||
changes.add(new FieldChange(entry, newFieldName, null, oldValue)); | ||
|
||
entry.clearField(oldFieldName); | ||
changes.add(new FieldChange(entry, oldFieldName, oldValue, null)); | ||
changes.add(entry.setField(newFieldName, oldValue).orElse(null)); | ||
changes.add(entry.clearField(oldFieldName).orElse(null)); | ||
} | ||
}); | ||
} | ||
|
||
// Dates: create date out of year and month, save it and delete old fields | ||
entry.getField(FieldName.DATE).ifPresent(date -> { | ||
if (date.isEmpty()) { | ||
entry.getFieldOrAlias(FieldName.DATE).ifPresent(newDate -> { | ||
Optional<String> oldYear = entry.getField(FieldName.YEAR); | ||
Optional<String> oldMonth = entry.getField(FieldName.MONTH); | ||
|
||
entry.setField(FieldName.DATE, newDate); | ||
entry.clearField(FieldName.YEAR); | ||
entry.clearField(FieldName.MONTH); | ||
|
||
changes.add(new FieldChange(entry, FieldName.DATE, null, newDate)); | ||
changes.add(new FieldChange(entry, FieldName.YEAR, oldYear.orElse(null), null)); | ||
changes.add(new FieldChange(entry, FieldName.MONTH, oldMonth.orElse(null), null)); | ||
}); | ||
} | ||
}); | ||
// If there already exists a value for the field date, it is not overwritten | ||
String date = ""; | ||
if (entry.getField(FieldName.DATE).isPresent()) { | ||
date = entry.getField(FieldName.DATE).get(); | ||
} | ||
if (StringUtils.isBlank(date)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. isBlank also accepts an optional I think (if not, please add a corresponding overload), hence |
||
entry.getFieldOrAlias(FieldName.DATE).ifPresent(newDate -> { | ||
changes.add(entry.setField(FieldName.DATE, newDate).orElse(null)); | ||
changes.add(entry.clearField(FieldName.YEAR).orElse(null)); | ||
changes.add(entry.clearField(FieldName.MONTH).orElse(null)); | ||
}); | ||
} | ||
return changes; | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
package net.sf.jabref.logic.cleanup; | ||
|
||
import java.util.Optional; | ||
|
||
import net.sf.jabref.model.entry.BibEntry; | ||
import net.sf.jabref.model.entry.FieldName; | ||
|
||
import org.junit.Assert; | ||
import org.junit.Before; | ||
import org.junit.Rule; | ||
import org.junit.Test; | ||
import org.junit.rules.TemporaryFolder; | ||
|
||
public class BiblatexCleanupTest { | ||
|
||
@Rule | ||
public TemporaryFolder bibFolder = new TemporaryFolder(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this variable ever used? |
||
|
||
private BiblatexCleanup worker; | ||
|
||
@Before | ||
public void setUp() { | ||
worker = new BiblatexCleanup(); | ||
} | ||
|
||
@Test | ||
public void convertToBiblatexMovesYearMonthToDate() { | ||
BibEntry entry = new BibEntry(); | ||
entry.setField(FieldName.YEAR, "2011"); | ||
entry.setField(FieldName.MONTH, "#jan#"); | ||
|
||
worker.cleanup(entry); | ||
Assert.assertEquals(Optional.empty(), entry.getField(FieldName.YEAR)); | ||
Assert.assertEquals(Optional.empty(), entry.getField(FieldName.MONTH)); | ||
Assert.assertEquals(Optional.of("2011-01"), entry.getField(FieldName.DATE)); | ||
} | ||
|
||
@Test | ||
public void convertToBiblatexDateAlreadyPresent() { | ||
BibEntry entry = new BibEntry(); | ||
entry.setField(FieldName.YEAR, "2011"); | ||
entry.setField(FieldName.MONTH, "#jan#"); | ||
entry.setField(FieldName.DATE, "2012"); | ||
|
||
worker.cleanup(entry); | ||
Assert.assertEquals(Optional.of("2011"), entry.getField(FieldName.YEAR)); | ||
Assert.assertEquals(Optional.of("#jan#"), entry.getField(FieldName.MONTH)); | ||
Assert.assertEquals(Optional.of("2012"), entry.getField(FieldName.DATE)); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,6 +24,7 @@ | |
import net.sf.jabref.model.database.BibDatabase; | ||
import net.sf.jabref.model.database.BibDatabaseContext; | ||
import net.sf.jabref.model.entry.BibEntry; | ||
import net.sf.jabref.model.entry.FieldName; | ||
import net.sf.jabref.model.entry.FileField; | ||
import net.sf.jabref.model.entry.ParsedFileField; | ||
import net.sf.jabref.model.metadata.MetaData; | ||
|
@@ -48,7 +49,6 @@ public class CleanupWorkerTest { | |
private CleanupWorker worker; | ||
private File pdfFolder; | ||
|
||
|
||
@Before | ||
public void setUp() throws IOException { | ||
pdfFolder = bibFolder.newFolder(); | ||
|
@@ -288,7 +288,8 @@ public void cleanupCasesAddsBracketAroundAluminiumGalliumArsenid() { | |
Collections.emptyList(), Collections.emptyList())); | ||
Assert.assertNotEquals(Collections.emptyList(), protectedTermsLoader.getProtectedTerms()); | ||
CleanupPreset preset = new CleanupPreset(new FieldFormatterCleanups(true, | ||
Collections.singletonList(new FieldFormatterCleanup("title", new ProtectTermsFormatter(protectedTermsLoader))))); | ||
Collections.singletonList( | ||
new FieldFormatterCleanup("title", new ProtectTermsFormatter(protectedTermsLoader))))); | ||
BibEntry entry = new BibEntry(); | ||
entry.setField("title", "AlGaAs"); | ||
|
||
|
@@ -311,11 +312,11 @@ public void cleanupLatexMergesTwoLatexMathEnvironments() { | |
public void convertToBiblatexMovesJournalToJournalTitle() { | ||
CleanupPreset preset = new CleanupPreset(CleanupPreset.CleanupStep.CONVERT_TO_BIBLATEX); | ||
BibEntry entry = new BibEntry(); | ||
entry.setField("journal", "test"); | ||
entry.setField(FieldName.JOURNAL, "test"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please revert these changes, as hard-coded strings in tests are actually desirable. The logic for the conversation moves FieldName.JOURNAL to FieldName,JOURNALTITLE. Hence if somebody changes |
||
|
||
worker.cleanup(preset, entry); | ||
Assert.assertEquals(Optional.empty(), entry.getField("journal")); | ||
Assert.assertEquals(Optional.of("test"), entry.getField("journaltitle")); | ||
Assert.assertEquals(Optional.empty(), entry.getField(FieldName.JOURNAL)); | ||
Assert.assertEquals(Optional.of("test"), entry.getField(FieldName.JOURNALTITLE)); | ||
} | ||
|
||
@Test | ||
|
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.
entry.setField(...).ifPresent(changes::add), otherwise you add null values to the list which is not so nice.