From 80a071f44f080f16ab96d8920347bbc5a7a4c378 Mon Sep 17 00:00:00 2001 From: Stefan Kolb Date: Tue, 27 Oct 2015 17:04:09 +0100 Subject: [PATCH 1/7] Add onSave action for pages fields #131 --- .../jabref/logic/cleanup/AutoFormatter.java | 55 ++++++++++++++ .../logic/cleanup/AutoFormatterTest.java | 76 +++++++++++++++++++ 2 files changed, 131 insertions(+) create mode 100644 src/main/java/net/sf/jabref/logic/cleanup/AutoFormatter.java create mode 100644 src/test/java/net/sf/jabref/logic/cleanup/AutoFormatterTest.java diff --git a/src/main/java/net/sf/jabref/logic/cleanup/AutoFormatter.java b/src/main/java/net/sf/jabref/logic/cleanup/AutoFormatter.java new file mode 100644 index 00000000000..b13bf09061f --- /dev/null +++ b/src/main/java/net/sf/jabref/logic/cleanup/AutoFormatter.java @@ -0,0 +1,55 @@ +package net.sf.jabref.logic.cleanup; + +import net.sf.jabref.model.entry.BibtexEntry; + +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +/** + * This class includes sensible defaults for consistent formatting of BibTex entries. + */ +public class AutoFormatter { + private BibtexEntry entry; + + public AutoFormatter(BibtexEntry entry) { + this.entry = entry; + } + + /** + * Runs all default cleanups for the BibTex entry. + */ + public void runDefaultCleanups() { + formatPageNumbers(); + } + + /** + * Format page numbers, separated either by commas or double-hyphens. + * Converts the range number format of the pages field to page_number--page_number. + * Keeps the existing String if the field does not match the expected Regex. + * + * + * 1-2 -> 1--2 + * 1,2,3 -> 1,2,3 + * Invalid -> Invalid + * + */ + public void formatPageNumbers() { + final String field = "pages"; + final Pattern pattern = Pattern.compile("\\A\\s*(\\d+)\\s*-{1,2}\\s*(\\d+)\\s*\\Z"); + final String replace = "$1--$2"; + + String value = entry.getField(field); + + // nothing to do + if (value == null || value.isEmpty()) { + return; + } + + Matcher matcher = pattern.matcher(value); + // replace + String newValue = matcher.replaceFirst(replace); + + // write field + entry.setField(field, newValue); + } +} diff --git a/src/test/java/net/sf/jabref/logic/cleanup/AutoFormatterTest.java b/src/test/java/net/sf/jabref/logic/cleanup/AutoFormatterTest.java new file mode 100644 index 00000000000..d3eb70e0ba1 --- /dev/null +++ b/src/test/java/net/sf/jabref/logic/cleanup/AutoFormatterTest.java @@ -0,0 +1,76 @@ +package net.sf.jabref.logic.cleanup; + +import junit.framework.Assert; +import net.sf.jabref.model.entry.BibtexEntry; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import static org.junit.Assert.*; + +public class AutoFormatterTest { + private BibtexEntry entry; + + @Before + public void setUp() { + entry = new BibtexEntry(); + } + + @After + public void teardown() { + entry = null; + } + + @Test + public void formatPageNumbers() { + entry.setField("pages", "1-2"); + new AutoFormatter(entry).formatPageNumbers(); + + Assert.assertEquals("1--2", entry.getField("pages")); + } + + @Test + public void formatPageNumbersCommaSeparated() { + entry.setField("pages", "1,2,3"); + new AutoFormatter(entry).formatPageNumbers(); + + Assert.assertEquals("1,2,3", entry.getField("pages")); + } + + @Test + public void ignoreWhitespaceInPageNumbers() { + entry.setField("pages", " 1 - 2 "); + new AutoFormatter(entry).formatPageNumbers(); + + Assert.assertEquals("1--2", entry.getField("pages")); + } + + @Test + public void onlyFormatPageNumbersField() { + entry.setField("otherfield", "1-2"); + new AutoFormatter(entry).formatPageNumbers(); + + Assert.assertEquals("1-2", entry.getField("otherfield")); + } + + @Test + public void formatPageNumbersEmptyFields() { + entry.setField("pages", ""); + new AutoFormatter(entry).formatPageNumbers(); + + Assert.assertEquals("", entry.getField("pages")); + + entry.setField("pages", null); + new AutoFormatter(entry).formatPageNumbers(); + + Assert.assertEquals(null, entry.getField("pages")); + } + + @Test + public void formatPageNumbersRegexNotMatching() { + entry.setField("pages", "12"); + new AutoFormatter(entry).formatPageNumbers(); + + Assert.assertEquals("12", entry.getField("pages")); + } +} \ No newline at end of file From 99bfb778320b2821c8043903732d61e3cf6d4d5c Mon Sep 17 00:00:00 2001 From: Stefan Kolb Date: Tue, 27 Oct 2015 17:24:00 +0100 Subject: [PATCH 2/7] Remove page number cleanup from CleanupAction as it is an onSave action now --- .../sf/jabref/gui/actions/CleanUpAction.java | 23 ------------------- 1 file changed, 23 deletions(-) diff --git a/src/main/java/net/sf/jabref/gui/actions/CleanUpAction.java b/src/main/java/net/sf/jabref/gui/actions/CleanUpAction.java index 94cd4f0e4e7..d535313c997 100644 --- a/src/main/java/net/sf/jabref/gui/actions/CleanUpAction.java +++ b/src/main/java/net/sf/jabref/gui/actions/CleanUpAction.java @@ -53,7 +53,6 @@ public class CleanUpAction extends AbstractWorker { private static final String AKS_AUTO_NAMING_PDFS_AGAIN = "AskAutoNamingPDFsAgain"; private static final String CLEANUP_DOI = "CleanUpDOI"; private static final String CLEANUP_MONTH = "CleanUpMonth"; - private static final String CLEANUP_PAGENUMBERS = "CleanUpPageNumbers"; private static final String CLEANUP_MAKEPATHSRELATIVE = "CleanUpMakePathsRelative"; private static final String CLEANUP_RENAMEPDF = "CleanUpRenamePDF"; private static final String CLEANUP_RENAMEPDF_ONLYRELATIVE_PATHS = "CleanUpRenamePDFonlyRelativePaths"; @@ -71,7 +70,6 @@ public static void putDefaults(HashMap defaults) { defaults.put(CLEANUP_SUPERSCRIPTS, Boolean.TRUE); defaults.put(CLEANUP_DOI, Boolean.TRUE); defaults.put(CLEANUP_MONTH, Boolean.TRUE); - defaults.put(CLEANUP_PAGENUMBERS, Boolean.TRUE); defaults.put(CLEANUP_MAKEPATHSRELATIVE, Boolean.TRUE); defaults.put(CLEANUP_RENAMEPDF, Boolean.TRUE); defaults.put(CLEANUP_RENAMEPDF_ONLYRELATIVE_PATHS, Boolean.FALSE); @@ -88,7 +86,6 @@ public static void putDefaults(HashMap defaults) { private JCheckBox cleanUpSuperscripts; private JCheckBox cleanUpDOI; private JCheckBox cleanUpMonth; - private JCheckBox cleanUpPageNumbers; private JCheckBox cleanUpMakePathsRelative; private JCheckBox cleanUpRenamePDF; private JCheckBox cleanUpRenamePDFonlyRelativePaths; @@ -124,7 +121,6 @@ private void initOptionsPanel() { cleanUpSuperscripts = new JCheckBox(Localization.lang("Convert 1st, 2nd, ... to real superscripts")); cleanUpDOI = new JCheckBox(Localization.lang("Move DOIs from note and URL field to DOI field and remove http prefix")); cleanUpMonth = new JCheckBox(Localization.lang("Format content of month field to #mon#")); - cleanUpPageNumbers = new JCheckBox(Localization.lang("Ensure that page ranges are of the form num1--num2")); cleanUpMakePathsRelative = new JCheckBox(Localization.lang("Make paths of linked files relative (if possible)")); cleanUpRenamePDF = new JCheckBox(Localization.lang("Rename PDFs to given filename format pattern")); cleanUpRenamePDF.addChangeListener(new ChangeListener() { @@ -154,7 +150,6 @@ public void stateChanged(ChangeEvent arg0) { builder.add(cleanUpSuperscripts).xyw(1, 6, 2); builder.add(cleanUpDOI).xyw(1, 7, 2); builder.add(cleanUpMonth).xyw(1, 8, 2); - builder.add(cleanUpPageNumbers).xyw(1, 9, 2); builder.add(cleanUpUpgradeExternalLinks).xyw(1, 10, 2); builder.add(cleanUpMakePathsRelative).xyw(1, 11, 2); builder.add(cleanUpRenamePDF).xyw(1, 12, 2); @@ -170,7 +165,6 @@ private void retrieveSettings() { cleanUpSuperscripts.setSelected(Globals.prefs.getBoolean(CleanUpAction.CLEANUP_SUPERSCRIPTS)); cleanUpDOI.setSelected(Globals.prefs.getBoolean(CleanUpAction.CLEANUP_DOI)); cleanUpMonth.setSelected(Globals.prefs.getBoolean(CleanUpAction.CLEANUP_MONTH)); - cleanUpPageNumbers.setSelected(Globals.prefs.getBoolean(CleanUpAction.CLEANUP_PAGENUMBERS)); cleanUpMakePathsRelative.setSelected(Globals.prefs.getBoolean(CleanUpAction.CLEANUP_MAKEPATHSRELATIVE)); cleanUpRenamePDF.setSelected(Globals.prefs.getBoolean(CleanUpAction.CLEANUP_RENAMEPDF)); cleanUpRenamePDFonlyRelativePaths.setSelected(Globals.prefs.getBoolean(CleanUpAction.CLEANUP_RENAMEPDF_ONLYRELATIVE_PATHS)); @@ -188,7 +182,6 @@ private void storeSettings() { Globals.prefs.putBoolean(CleanUpAction.CLEANUP_SUPERSCRIPTS, cleanUpSuperscripts.isSelected()); Globals.prefs.putBoolean(CleanUpAction.CLEANUP_DOI, cleanUpDOI.isSelected()); Globals.prefs.putBoolean(CleanUpAction.CLEANUP_MONTH, cleanUpMonth.isSelected()); - Globals.prefs.putBoolean(CleanUpAction.CLEANUP_PAGENUMBERS, cleanUpPageNumbers.isSelected()); Globals.prefs.putBoolean(CleanUpAction.CLEANUP_MAKEPATHSRELATIVE, cleanUpMakePathsRelative.isSelected()); Globals.prefs.putBoolean(CleanUpAction.CLEANUP_RENAMEPDF, cleanUpRenamePDF.isSelected()); Globals.prefs.putBoolean(CleanUpAction.CLEANUP_RENAMEPDF_ONLYRELATIVE_PATHS, cleanUpRenamePDFonlyRelativePaths.isSelected()); @@ -243,7 +236,6 @@ public void run() { boolean choiceCleanUpSuperscripts = cleanUpSuperscripts.isSelected(); boolean choiceCleanUpDOI = cleanUpDOI.isSelected(); boolean choiceCleanUpMonth = cleanUpMonth.isSelected(); - boolean choiceCleanUpPageNumbers = cleanUpPageNumbers.isSelected(); boolean choiceCleanUpUpgradeExternalLinks = cleanUpUpgradeExternalLinks.isSelected(); boolean choiceMakePathsRelative = cleanUpMakePathsRelative.isSelected(); boolean choiceRenamePDF = cleanUpRenamePDF.isSelected(); @@ -293,9 +285,6 @@ public void run() { if (choiceCleanUpMonth) { doCleanUpMonth(entry, ce); } - if (choiceCleanUpPageNumbers) { - doCleanUpPageNumbers(entry, ce); - } fixWrongFileEntries(entry, ce); if (choiceMakePathsRelative) { doMakePathsRelative(entry, ce); @@ -443,18 +432,6 @@ private static void doCleanUpMonth(BibtexEntry entry, NamedCompound ce) { } } - private static void doCleanUpPageNumbers(BibtexEntry entry, NamedCompound ce) { - String oldValue = entry.getField("pages"); - if (oldValue == null) { - return; - } - String newValue = oldValue.replaceAll(" *(\\d+) *- *(\\d+) *", "$1--$2"); - if (!oldValue.equals(newValue)) { - entry.setField("pages", newValue); - ce.addEdit(new UndoableFieldChange(entry, "pages", oldValue, newValue)); - } - } - private static void fixWrongFileEntries(BibtexEntry entry, NamedCompound ce) { String oldValue = entry.getField(Globals.FILE_FIELD); if (oldValue == null) { From 9a6d022e10c268d627bff584b7d3719d91732df8 Mon Sep 17 00:00:00 2001 From: Stefan Kolb Date: Wed, 28 Oct 2015 15:32:28 +0100 Subject: [PATCH 3/7] Revert "Remove page number cleanup from CleanupAction as it is an onSave action now" This reverts commit 99bfb778320b2821c8043903732d61e3cf6d4d5c. --- .../sf/jabref/gui/actions/CleanUpAction.java | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/main/java/net/sf/jabref/gui/actions/CleanUpAction.java b/src/main/java/net/sf/jabref/gui/actions/CleanUpAction.java index d535313c997..94cd4f0e4e7 100644 --- a/src/main/java/net/sf/jabref/gui/actions/CleanUpAction.java +++ b/src/main/java/net/sf/jabref/gui/actions/CleanUpAction.java @@ -53,6 +53,7 @@ public class CleanUpAction extends AbstractWorker { private static final String AKS_AUTO_NAMING_PDFS_AGAIN = "AskAutoNamingPDFsAgain"; private static final String CLEANUP_DOI = "CleanUpDOI"; private static final String CLEANUP_MONTH = "CleanUpMonth"; + private static final String CLEANUP_PAGENUMBERS = "CleanUpPageNumbers"; private static final String CLEANUP_MAKEPATHSRELATIVE = "CleanUpMakePathsRelative"; private static final String CLEANUP_RENAMEPDF = "CleanUpRenamePDF"; private static final String CLEANUP_RENAMEPDF_ONLYRELATIVE_PATHS = "CleanUpRenamePDFonlyRelativePaths"; @@ -70,6 +71,7 @@ public static void putDefaults(HashMap defaults) { defaults.put(CLEANUP_SUPERSCRIPTS, Boolean.TRUE); defaults.put(CLEANUP_DOI, Boolean.TRUE); defaults.put(CLEANUP_MONTH, Boolean.TRUE); + defaults.put(CLEANUP_PAGENUMBERS, Boolean.TRUE); defaults.put(CLEANUP_MAKEPATHSRELATIVE, Boolean.TRUE); defaults.put(CLEANUP_RENAMEPDF, Boolean.TRUE); defaults.put(CLEANUP_RENAMEPDF_ONLYRELATIVE_PATHS, Boolean.FALSE); @@ -86,6 +88,7 @@ public static void putDefaults(HashMap defaults) { private JCheckBox cleanUpSuperscripts; private JCheckBox cleanUpDOI; private JCheckBox cleanUpMonth; + private JCheckBox cleanUpPageNumbers; private JCheckBox cleanUpMakePathsRelative; private JCheckBox cleanUpRenamePDF; private JCheckBox cleanUpRenamePDFonlyRelativePaths; @@ -121,6 +124,7 @@ private void initOptionsPanel() { cleanUpSuperscripts = new JCheckBox(Localization.lang("Convert 1st, 2nd, ... to real superscripts")); cleanUpDOI = new JCheckBox(Localization.lang("Move DOIs from note and URL field to DOI field and remove http prefix")); cleanUpMonth = new JCheckBox(Localization.lang("Format content of month field to #mon#")); + cleanUpPageNumbers = new JCheckBox(Localization.lang("Ensure that page ranges are of the form num1--num2")); cleanUpMakePathsRelative = new JCheckBox(Localization.lang("Make paths of linked files relative (if possible)")); cleanUpRenamePDF = new JCheckBox(Localization.lang("Rename PDFs to given filename format pattern")); cleanUpRenamePDF.addChangeListener(new ChangeListener() { @@ -150,6 +154,7 @@ public void stateChanged(ChangeEvent arg0) { builder.add(cleanUpSuperscripts).xyw(1, 6, 2); builder.add(cleanUpDOI).xyw(1, 7, 2); builder.add(cleanUpMonth).xyw(1, 8, 2); + builder.add(cleanUpPageNumbers).xyw(1, 9, 2); builder.add(cleanUpUpgradeExternalLinks).xyw(1, 10, 2); builder.add(cleanUpMakePathsRelative).xyw(1, 11, 2); builder.add(cleanUpRenamePDF).xyw(1, 12, 2); @@ -165,6 +170,7 @@ private void retrieveSettings() { cleanUpSuperscripts.setSelected(Globals.prefs.getBoolean(CleanUpAction.CLEANUP_SUPERSCRIPTS)); cleanUpDOI.setSelected(Globals.prefs.getBoolean(CleanUpAction.CLEANUP_DOI)); cleanUpMonth.setSelected(Globals.prefs.getBoolean(CleanUpAction.CLEANUP_MONTH)); + cleanUpPageNumbers.setSelected(Globals.prefs.getBoolean(CleanUpAction.CLEANUP_PAGENUMBERS)); cleanUpMakePathsRelative.setSelected(Globals.prefs.getBoolean(CleanUpAction.CLEANUP_MAKEPATHSRELATIVE)); cleanUpRenamePDF.setSelected(Globals.prefs.getBoolean(CleanUpAction.CLEANUP_RENAMEPDF)); cleanUpRenamePDFonlyRelativePaths.setSelected(Globals.prefs.getBoolean(CleanUpAction.CLEANUP_RENAMEPDF_ONLYRELATIVE_PATHS)); @@ -182,6 +188,7 @@ private void storeSettings() { Globals.prefs.putBoolean(CleanUpAction.CLEANUP_SUPERSCRIPTS, cleanUpSuperscripts.isSelected()); Globals.prefs.putBoolean(CleanUpAction.CLEANUP_DOI, cleanUpDOI.isSelected()); Globals.prefs.putBoolean(CleanUpAction.CLEANUP_MONTH, cleanUpMonth.isSelected()); + Globals.prefs.putBoolean(CleanUpAction.CLEANUP_PAGENUMBERS, cleanUpPageNumbers.isSelected()); Globals.prefs.putBoolean(CleanUpAction.CLEANUP_MAKEPATHSRELATIVE, cleanUpMakePathsRelative.isSelected()); Globals.prefs.putBoolean(CleanUpAction.CLEANUP_RENAMEPDF, cleanUpRenamePDF.isSelected()); Globals.prefs.putBoolean(CleanUpAction.CLEANUP_RENAMEPDF_ONLYRELATIVE_PATHS, cleanUpRenamePDFonlyRelativePaths.isSelected()); @@ -236,6 +243,7 @@ public void run() { boolean choiceCleanUpSuperscripts = cleanUpSuperscripts.isSelected(); boolean choiceCleanUpDOI = cleanUpDOI.isSelected(); boolean choiceCleanUpMonth = cleanUpMonth.isSelected(); + boolean choiceCleanUpPageNumbers = cleanUpPageNumbers.isSelected(); boolean choiceCleanUpUpgradeExternalLinks = cleanUpUpgradeExternalLinks.isSelected(); boolean choiceMakePathsRelative = cleanUpMakePathsRelative.isSelected(); boolean choiceRenamePDF = cleanUpRenamePDF.isSelected(); @@ -285,6 +293,9 @@ public void run() { if (choiceCleanUpMonth) { doCleanUpMonth(entry, ce); } + if (choiceCleanUpPageNumbers) { + doCleanUpPageNumbers(entry, ce); + } fixWrongFileEntries(entry, ce); if (choiceMakePathsRelative) { doMakePathsRelative(entry, ce); @@ -432,6 +443,18 @@ private static void doCleanUpMonth(BibtexEntry entry, NamedCompound ce) { } } + private static void doCleanUpPageNumbers(BibtexEntry entry, NamedCompound ce) { + String oldValue = entry.getField("pages"); + if (oldValue == null) { + return; + } + String newValue = oldValue.replaceAll(" *(\\d+) *- *(\\d+) *", "$1--$2"); + if (!oldValue.equals(newValue)) { + entry.setField("pages", newValue); + ce.addEdit(new UndoableFieldChange(entry, "pages", oldValue, newValue)); + } + } + private static void fixWrongFileEntries(BibtexEntry entry, NamedCompound ce) { String oldValue = entry.getField(Globals.FILE_FIELD); if (oldValue == null) { From 83ca0dc054e34597d7ab137efb060371bf4fc2b6 Mon Sep 17 00:00:00 2001 From: Stefan Kolb Date: Wed, 28 Oct 2015 16:02:14 +0100 Subject: [PATCH 4/7] Only replace old autocleanup action for now --- .../sf/jabref/gui/actions/CleanUpAction.java | 15 ++++---- .../PageNumbersFormatter.java} | 37 ++++++++++--------- .../PageNumbersFormatterTest.java} | 36 ++++++++++++------ 3 files changed, 51 insertions(+), 37 deletions(-) rename src/main/java/net/sf/jabref/logic/{cleanup/AutoFormatter.java => formatter/PageNumbersFormatter.java} (51%) rename src/test/java/net/sf/jabref/logic/{cleanup/AutoFormatterTest.java => formatter/PageNumbersFormatterTest.java} (62%) diff --git a/src/main/java/net/sf/jabref/gui/actions/CleanUpAction.java b/src/main/java/net/sf/jabref/gui/actions/CleanUpAction.java index 94cd4f0e4e7..db568b485c0 100644 --- a/src/main/java/net/sf/jabref/gui/actions/CleanUpAction.java +++ b/src/main/java/net/sf/jabref/gui/actions/CleanUpAction.java @@ -41,6 +41,7 @@ import com.jgoodies.forms.builder.FormBuilder; import com.jgoodies.forms.layout.FormLayout; +import net.sf.jabref.logic.formatter.PageNumbersFormatter; import net.sf.jabref.logic.l10n.Localization; import net.sf.jabref.model.entry.BibtexEntry; import net.sf.jabref.logic.util.DOI; @@ -445,14 +446,12 @@ private static void doCleanUpMonth(BibtexEntry entry, NamedCompound ce) { private static void doCleanUpPageNumbers(BibtexEntry entry, NamedCompound ce) { String oldValue = entry.getField("pages"); - if (oldValue == null) { - return; - } - String newValue = oldValue.replaceAll(" *(\\d+) *- *(\\d+) *", "$1--$2"); - if (!oldValue.equals(newValue)) { - entry.setField("pages", newValue); - ce.addEdit(new UndoableFieldChange(entry, "pages", oldValue, newValue)); - } + // run formatter + new PageNumbersFormatter(entry).format(); + // new value + String newValue = entry.getField("pages"); + // undo action + ce.addEdit(new UndoableFieldChange(entry, "pages", oldValue, newValue)); } private static void fixWrongFileEntries(BibtexEntry entry, NamedCompound ce) { diff --git a/src/main/java/net/sf/jabref/logic/cleanup/AutoFormatter.java b/src/main/java/net/sf/jabref/logic/formatter/PageNumbersFormatter.java similarity index 51% rename from src/main/java/net/sf/jabref/logic/cleanup/AutoFormatter.java rename to src/main/java/net/sf/jabref/logic/formatter/PageNumbersFormatter.java index b13bf09061f..ae839154bbb 100644 --- a/src/main/java/net/sf/jabref/logic/cleanup/AutoFormatter.java +++ b/src/main/java/net/sf/jabref/logic/formatter/PageNumbersFormatter.java @@ -1,4 +1,4 @@ -package net.sf.jabref.logic.cleanup; +package net.sf.jabref.logic.formatter; import net.sf.jabref.model.entry.BibtexEntry; @@ -6,36 +6,32 @@ import java.util.regex.Pattern; /** - * This class includes sensible defaults for consistent formatting of BibTex entries. + * This class includes sensible defaults for consistent formatting of BibTex page numbers. */ -public class AutoFormatter { +public class PageNumbersFormatter { private BibtexEntry entry; - public AutoFormatter(BibtexEntry entry) { + public PageNumbersFormatter(BibtexEntry entry) { this.entry = entry; } - /** - * Runs all default cleanups for the BibTex entry. - */ - public void runDefaultCleanups() { - formatPageNumbers(); - } - /** * Format page numbers, separated either by commas or double-hyphens. * Converts the range number format of the pages field to page_number--page_number. - * Keeps the existing String if the field does not match the expected Regex. + * Removes all literals except [0-9,-]. + * Keeps the existing String if the resulting field does not match the expected Regex. * * * 1-2 -> 1--2 * 1,2,3 -> 1,2,3 + * {1}-{2} -> 1--2 * Invalid -> Invalid * */ - public void formatPageNumbers() { + public void format() { final String field = "pages"; - final Pattern pattern = Pattern.compile("\\A\\s*(\\d+)\\s*-{1,2}\\s*(\\d+)\\s*\\Z"); + final String rejectLiterals = "[^0-9,-]"; + final Pattern pagesPattern = Pattern.compile("\\A(\\d+)-{1,2}(\\d+)\\Z"); final String replace = "$1--$2"; String value = entry.getField(field); @@ -45,11 +41,16 @@ public void formatPageNumbers() { return; } - Matcher matcher = pattern.matcher(value); + // remove unwanted literals incl. whitespace + String cleanValue = value.replaceAll(rejectLiterals, ""); + // try to find pages pattern + Matcher matcher = pagesPattern.matcher(cleanValue); // replace String newValue = matcher.replaceFirst(replace); - - // write field - entry.setField(field, newValue); + // replacement? + if(!newValue.equals(cleanValue)) { + // write field + entry.setField(field, newValue); + } } } diff --git a/src/test/java/net/sf/jabref/logic/cleanup/AutoFormatterTest.java b/src/test/java/net/sf/jabref/logic/formatter/PageNumbersFormatterTest.java similarity index 62% rename from src/test/java/net/sf/jabref/logic/cleanup/AutoFormatterTest.java rename to src/test/java/net/sf/jabref/logic/formatter/PageNumbersFormatterTest.java index d3eb70e0ba1..0b3eb3e3f49 100644 --- a/src/test/java/net/sf/jabref/logic/cleanup/AutoFormatterTest.java +++ b/src/test/java/net/sf/jabref/logic/formatter/PageNumbersFormatterTest.java @@ -1,4 +1,4 @@ -package net.sf.jabref.logic.cleanup; +package net.sf.jabref.logic.formatter; import junit.framework.Assert; import net.sf.jabref.model.entry.BibtexEntry; @@ -6,9 +6,7 @@ import org.junit.Before; import org.junit.Test; -import static org.junit.Assert.*; - -public class AutoFormatterTest { +public class PageNumbersFormatterTest { private BibtexEntry entry; @Before @@ -24,7 +22,7 @@ public void teardown() { @Test public void formatPageNumbers() { entry.setField("pages", "1-2"); - new AutoFormatter(entry).formatPageNumbers(); + new PageNumbersFormatter(entry).format(); Assert.assertEquals("1--2", entry.getField("pages")); } @@ -32,7 +30,7 @@ public void formatPageNumbers() { @Test public void formatPageNumbersCommaSeparated() { entry.setField("pages", "1,2,3"); - new AutoFormatter(entry).formatPageNumbers(); + new PageNumbersFormatter(entry).format(); Assert.assertEquals("1,2,3", entry.getField("pages")); } @@ -40,7 +38,15 @@ public void formatPageNumbersCommaSeparated() { @Test public void ignoreWhitespaceInPageNumbers() { entry.setField("pages", " 1 - 2 "); - new AutoFormatter(entry).formatPageNumbers(); + new PageNumbersFormatter(entry).format(); + + Assert.assertEquals("1--2", entry.getField("pages")); + } + + @Test + public void keepCorrectlyFormattedPageNumbers() { + entry.setField("pages", "1--2"); + new PageNumbersFormatter(entry).format(); Assert.assertEquals("1--2", entry.getField("pages")); } @@ -48,7 +54,7 @@ public void ignoreWhitespaceInPageNumbers() { @Test public void onlyFormatPageNumbersField() { entry.setField("otherfield", "1-2"); - new AutoFormatter(entry).formatPageNumbers(); + new PageNumbersFormatter(entry).format(); Assert.assertEquals("1-2", entry.getField("otherfield")); } @@ -56,20 +62,28 @@ public void onlyFormatPageNumbersField() { @Test public void formatPageNumbersEmptyFields() { entry.setField("pages", ""); - new AutoFormatter(entry).formatPageNumbers(); + new PageNumbersFormatter(entry).format(); Assert.assertEquals("", entry.getField("pages")); entry.setField("pages", null); - new AutoFormatter(entry).formatPageNumbers(); + new PageNumbersFormatter(entry).format(); Assert.assertEquals(null, entry.getField("pages")); } + @Test + public void formatPageNumbersRemoveUnexpectedLiterals() { + entry.setField("pages", "{1}-{2}"); + new PageNumbersFormatter(entry).format(); + + Assert.assertEquals("1--2", entry.getField("pages")); + } + @Test public void formatPageNumbersRegexNotMatching() { entry.setField("pages", "12"); - new AutoFormatter(entry).formatPageNumbers(); + new PageNumbersFormatter(entry).format(); Assert.assertEquals("12", entry.getField("pages")); } From 66244850f948e013e298a36e1d1719de62addc56 Mon Sep 17 00:00:00 2001 From: Stefan Kolb Date: Wed, 28 Oct 2015 17:11:30 +0100 Subject: [PATCH 5/7] Use new interface paradigma for all field formatters --- .../sf/jabref/gui/actions/CleanUpAction.java | 4 +- .../logic/cleanup/PageNumbersCleanup.java | 30 +++++++++ .../logic/formatter/FieldFormatters.java | 10 +++ .../sf/jabref/logic/formatter/Formatter.java | 21 ++++++ .../logic/formatter/PageNumbersFormatter.java | 21 +++--- .../logic/cleanup/PageNumbersCleanupTest.java | 39 +++++++++++ .../formatter/PageNumbersFormatterTest.java | 65 +++++++++---------- 7 files changed, 139 insertions(+), 51 deletions(-) create mode 100644 src/main/java/net/sf/jabref/logic/cleanup/PageNumbersCleanup.java create mode 100644 src/main/java/net/sf/jabref/logic/formatter/FieldFormatters.java create mode 100644 src/main/java/net/sf/jabref/logic/formatter/Formatter.java create mode 100644 src/test/java/net/sf/jabref/logic/cleanup/PageNumbersCleanupTest.java diff --git a/src/main/java/net/sf/jabref/gui/actions/CleanUpAction.java b/src/main/java/net/sf/jabref/gui/actions/CleanUpAction.java index db568b485c0..b045fad495d 100644 --- a/src/main/java/net/sf/jabref/gui/actions/CleanUpAction.java +++ b/src/main/java/net/sf/jabref/gui/actions/CleanUpAction.java @@ -41,7 +41,7 @@ import com.jgoodies.forms.builder.FormBuilder; import com.jgoodies.forms.layout.FormLayout; -import net.sf.jabref.logic.formatter.PageNumbersFormatter; +import net.sf.jabref.logic.cleanup.PageNumbersCleanup; import net.sf.jabref.logic.l10n.Localization; import net.sf.jabref.model.entry.BibtexEntry; import net.sf.jabref.logic.util.DOI; @@ -447,7 +447,7 @@ private static void doCleanUpMonth(BibtexEntry entry, NamedCompound ce) { private static void doCleanUpPageNumbers(BibtexEntry entry, NamedCompound ce) { String oldValue = entry.getField("pages"); // run formatter - new PageNumbersFormatter(entry).format(); + new PageNumbersCleanup(entry).cleanup(); // new value String newValue = entry.getField("pages"); // undo action diff --git a/src/main/java/net/sf/jabref/logic/cleanup/PageNumbersCleanup.java b/src/main/java/net/sf/jabref/logic/cleanup/PageNumbersCleanup.java new file mode 100644 index 00000000000..7e7d197f1ba --- /dev/null +++ b/src/main/java/net/sf/jabref/logic/cleanup/PageNumbersCleanup.java @@ -0,0 +1,30 @@ +package net.sf.jabref.logic.cleanup; + +import net.sf.jabref.logic.formatter.FieldFormatters; +import net.sf.jabref.logic.formatter.PageNumbersFormatter; +import net.sf.jabref.model.entry.BibtexEntry; + +/** + * This class includes sensible defaults for consistent formatting of BibTex page numbers. + */ +public class PageNumbersCleanup { + private BibtexEntry entry; + + public PageNumbersCleanup(BibtexEntry entry) { + this.entry = entry; + } + + /** + * Format page numbers, separated either by commas or double-hyphens. + * Converts the range number format of the pages field to page_number--page_number. + * + * @see{PageNumbersFormatter} + */ + public void cleanup() { + final String field = "pages"; + + String value = entry.getField(field); + String newValue = FieldFormatters.PAGE_NUMBERS.format(value); + entry.setField(field, newValue); + } +} diff --git a/src/main/java/net/sf/jabref/logic/formatter/FieldFormatters.java b/src/main/java/net/sf/jabref/logic/formatter/FieldFormatters.java new file mode 100644 index 00000000000..9957ebd5829 --- /dev/null +++ b/src/main/java/net/sf/jabref/logic/formatter/FieldFormatters.java @@ -0,0 +1,10 @@ +package net.sf.jabref.logic.formatter; + +import java.util.Arrays; +import java.util.List; + +public class FieldFormatters { + public static final PageNumbersFormatter PAGE_NUMBERS = new PageNumbersFormatter(); + + public static final List ALL = Arrays.asList(PAGE_NUMBERS); +} diff --git a/src/main/java/net/sf/jabref/logic/formatter/Formatter.java b/src/main/java/net/sf/jabref/logic/formatter/Formatter.java new file mode 100644 index 00000000000..e55631184bd --- /dev/null +++ b/src/main/java/net/sf/jabref/logic/formatter/Formatter.java @@ -0,0 +1,21 @@ +package net.sf.jabref.logic.formatter; + +/** + * Formatter Interface + */ +public interface Formatter { + /** + * Returns a human readable name of the formatter usable for e.g. in the GUI + * + * @return the name of the formatter + */ + String getName(); + + /** + * Formats a field value by with a particular formatter transformation. + * + * @param value the input String + * @return the formatted output String + */ + String format(String value); +} diff --git a/src/main/java/net/sf/jabref/logic/formatter/PageNumbersFormatter.java b/src/main/java/net/sf/jabref/logic/formatter/PageNumbersFormatter.java index ae839154bbb..00f81e918b4 100644 --- a/src/main/java/net/sf/jabref/logic/formatter/PageNumbersFormatter.java +++ b/src/main/java/net/sf/jabref/logic/formatter/PageNumbersFormatter.java @@ -1,18 +1,15 @@ package net.sf.jabref.logic.formatter; -import net.sf.jabref.model.entry.BibtexEntry; - import java.util.regex.Matcher; import java.util.regex.Pattern; /** * This class includes sensible defaults for consistent formatting of BibTex page numbers. */ -public class PageNumbersFormatter { - private BibtexEntry entry; - - public PageNumbersFormatter(BibtexEntry entry) { - this.entry = entry; +public class PageNumbersFormatter implements Formatter { + @Override + public String getName() { + return "Page numbers"; } /** @@ -28,17 +25,14 @@ public PageNumbersFormatter(BibtexEntry entry) { * Invalid -> Invalid * */ - public void format() { - final String field = "pages"; + public String format(String value) { final String rejectLiterals = "[^0-9,-]"; final Pattern pagesPattern = Pattern.compile("\\A(\\d+)-{1,2}(\\d+)\\Z"); final String replace = "$1--$2"; - String value = entry.getField(field); - // nothing to do if (value == null || value.isEmpty()) { - return; + return value; } // remove unwanted literals incl. whitespace @@ -50,7 +44,8 @@ public void format() { // replacement? if(!newValue.equals(cleanValue)) { // write field - entry.setField(field, newValue); + return newValue; } + return value; } } diff --git a/src/test/java/net/sf/jabref/logic/cleanup/PageNumbersCleanupTest.java b/src/test/java/net/sf/jabref/logic/cleanup/PageNumbersCleanupTest.java new file mode 100644 index 00000000000..53ededba9ef --- /dev/null +++ b/src/test/java/net/sf/jabref/logic/cleanup/PageNumbersCleanupTest.java @@ -0,0 +1,39 @@ +package net.sf.jabref.logic.cleanup; + +import junit.framework.Assert; +import net.sf.jabref.model.entry.BibtexEntry; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import static org.junit.Assert.*; + +public class PageNumbersCleanupTest { + private BibtexEntry entry; + + @Before + public void setUp() { + entry = new BibtexEntry(); + } + + @After + public void teardown() { + entry = null; + } + + @Test + public void formatPageNumbers() { + entry.setField("pages", "1-2"); + new PageNumbersCleanup(entry).cleanup(); + + Assert.assertEquals("1--2", entry.getField("pages")); + } + + @Test + public void onlyFormatPageNumbersField() { + entry.setField("otherfield", "1-2"); + new PageNumbersCleanup(entry).cleanup(); + + Assert.assertEquals("1-2", entry.getField("otherfield")); + } +} \ No newline at end of file diff --git a/src/test/java/net/sf/jabref/logic/formatter/PageNumbersFormatterTest.java b/src/test/java/net/sf/jabref/logic/formatter/PageNumbersFormatterTest.java index 0b3eb3e3f49..55a593684fb 100644 --- a/src/test/java/net/sf/jabref/logic/formatter/PageNumbersFormatterTest.java +++ b/src/test/java/net/sf/jabref/logic/formatter/PageNumbersFormatterTest.java @@ -1,90 +1,83 @@ package net.sf.jabref.logic.formatter; import junit.framework.Assert; -import net.sf.jabref.model.entry.BibtexEntry; import org.junit.After; import org.junit.Before; import org.junit.Test; +import static org.junit.Assert.*; + public class PageNumbersFormatterTest { - private BibtexEntry entry; + private PageNumbersFormatter formatter; @Before public void setUp() { - entry = new BibtexEntry(); + formatter = new PageNumbersFormatter(); } @After public void teardown() { - entry = null; + formatter = null; } @Test public void formatPageNumbers() { - entry.setField("pages", "1-2"); - new PageNumbersFormatter(entry).format(); + String value = "1-2"; + String result = formatter.format(value); - Assert.assertEquals("1--2", entry.getField("pages")); + Assert.assertEquals("1--2", result); } @Test public void formatPageNumbersCommaSeparated() { - entry.setField("pages", "1,2,3"); - new PageNumbersFormatter(entry).format(); + String value = "1,2,3"; + String result = formatter.format(value); - Assert.assertEquals("1,2,3", entry.getField("pages")); + Assert.assertEquals("1,2,3", result); } @Test public void ignoreWhitespaceInPageNumbers() { - entry.setField("pages", " 1 - 2 "); - new PageNumbersFormatter(entry).format(); + String value = " 1 - 2 "; + String result = formatter.format(value); - Assert.assertEquals("1--2", entry.getField("pages")); + Assert.assertEquals("1--2", result); } @Test public void keepCorrectlyFormattedPageNumbers() { - entry.setField("pages", "1--2"); - new PageNumbersFormatter(entry).format(); - - Assert.assertEquals("1--2", entry.getField("pages")); - } - - @Test - public void onlyFormatPageNumbersField() { - entry.setField("otherfield", "1-2"); - new PageNumbersFormatter(entry).format(); + String value = "1--2"; + String result = formatter.format(value); - Assert.assertEquals("1-2", entry.getField("otherfield")); + Assert.assertEquals("1--2", result); } @Test public void formatPageNumbersEmptyFields() { - entry.setField("pages", ""); - new PageNumbersFormatter(entry).format(); + String value = ""; + String result = formatter.format(value); - Assert.assertEquals("", entry.getField("pages")); + Assert.assertEquals("", result); - entry.setField("pages", null); - new PageNumbersFormatter(entry).format(); + value = null; + result = formatter.format(value); - Assert.assertEquals(null, entry.getField("pages")); + Assert.assertEquals(null, result); } @Test public void formatPageNumbersRemoveUnexpectedLiterals() { - entry.setField("pages", "{1}-{2}"); - new PageNumbersFormatter(entry).format(); + String value = "{1}-{2}"; + String result = formatter.format(value); - Assert.assertEquals("1--2", entry.getField("pages")); + Assert.assertEquals("1--2", result); } @Test public void formatPageNumbersRegexNotMatching() { - entry.setField("pages", "12"); - new PageNumbersFormatter(entry).format(); + String value = "12"; + String result = formatter.format(value); - Assert.assertEquals("12", entry.getField("pages")); + Assert.assertEquals("12", result); } } \ No newline at end of file From cc5db9f47cccce77e06854be7028c3035f7473ca Mon Sep 17 00:00:00 2001 From: Stefan Kolb Date: Wed, 28 Oct 2015 17:27:58 +0100 Subject: [PATCH 6/7] Refactor PageNumbersFormatter test --- .../formatter/PageNumbersFormatterTest.java | 43 +++++-------------- 1 file changed, 11 insertions(+), 32 deletions(-) diff --git a/src/test/java/net/sf/jabref/logic/formatter/PageNumbersFormatterTest.java b/src/test/java/net/sf/jabref/logic/formatter/PageNumbersFormatterTest.java index 55a593684fb..d3decb3b106 100644 --- a/src/test/java/net/sf/jabref/logic/formatter/PageNumbersFormatterTest.java +++ b/src/test/java/net/sf/jabref/logic/formatter/PageNumbersFormatterTest.java @@ -22,62 +22,41 @@ public void teardown() { @Test public void formatPageNumbers() { - String value = "1-2"; - String result = formatter.format(value); - - Assert.assertEquals("1--2", result); + expectCorrect("1-2", "1--2"); } @Test public void formatPageNumbersCommaSeparated() { - String value = "1,2,3"; - String result = formatter.format(value); - - Assert.assertEquals("1,2,3", result); + expectCorrect("1,2,3", "1,2,3"); } @Test public void ignoreWhitespaceInPageNumbers() { - String value = " 1 - 2 "; - String result = formatter.format(value); - - Assert.assertEquals("1--2", result); + expectCorrect(" 1 - 2 ", "1--2"); } @Test public void keepCorrectlyFormattedPageNumbers() { - String value = "1--2"; - String result = formatter.format(value); - - Assert.assertEquals("1--2", result); + expectCorrect("1--2", "1--2"); } @Test public void formatPageNumbersEmptyFields() { - String value = ""; - String result = formatter.format(value); - - Assert.assertEquals("", result); - - value = null; - result = formatter.format(value); - - Assert.assertEquals(null, result); + expectCorrect("", ""); + expectCorrect(null, null); } @Test public void formatPageNumbersRemoveUnexpectedLiterals() { - String value = "{1}-{2}"; - String result = formatter.format(value); - - Assert.assertEquals("1--2", result); + expectCorrect("{1}-{2}", "1--2"); } @Test public void formatPageNumbersRegexNotMatching() { - String value = "12"; - String result = formatter.format(value); + expectCorrect("12", "12"); + } - Assert.assertEquals("12", result); + private void expectCorrect(String input, String expected) { + Assert.assertEquals(expected, formatter.format(input)); } } \ No newline at end of file From 60811a514a9d430c4cb060b319d65e3489e39cd3 Mon Sep 17 00:00:00 2001 From: Stefan Kolb Date: Thu, 29 Oct 2015 09:50:28 +0100 Subject: [PATCH 7/7] Only create the undo action if value was changed --- src/main/java/net/sf/jabref/gui/actions/CleanUpAction.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/main/java/net/sf/jabref/gui/actions/CleanUpAction.java b/src/main/java/net/sf/jabref/gui/actions/CleanUpAction.java index b045fad495d..6491ebe6d9e 100644 --- a/src/main/java/net/sf/jabref/gui/actions/CleanUpAction.java +++ b/src/main/java/net/sf/jabref/gui/actions/CleanUpAction.java @@ -42,6 +42,7 @@ import com.jgoodies.forms.builder.FormBuilder; import com.jgoodies.forms.layout.FormLayout; import net.sf.jabref.logic.cleanup.PageNumbersCleanup; +import net.sf.jabref.logic.formatter.FieldFormatters; import net.sf.jabref.logic.l10n.Localization; import net.sf.jabref.model.entry.BibtexEntry; import net.sf.jabref.logic.util.DOI; @@ -451,7 +452,9 @@ private static void doCleanUpPageNumbers(BibtexEntry entry, NamedCompound ce) { // new value String newValue = entry.getField("pages"); // undo action - ce.addEdit(new UndoableFieldChange(entry, "pages", oldValue, newValue)); + if(!oldValue.equals(newValue)) { + ce.addEdit(new UndoableFieldChange(entry, "pages", oldValue, newValue)); + } } private static void fixWrongFileEntries(BibtexEntry entry, NamedCompound ce) {