From fc844dd4735a1910966e1a5d13f041318e7bbeee Mon Sep 17 00:00:00 2001 From: ali_zhagparov Date: Thu, 4 Mar 2021 21:54:22 +0300 Subject: [PATCH 01/11] check with regex instead of throwing exception --- .../comparator/NumericFieldComparator.java | 44 ++++++------------- 1 file changed, 13 insertions(+), 31 deletions(-) diff --git a/src/main/java/org/jabref/gui/util/comparator/NumericFieldComparator.java b/src/main/java/org/jabref/gui/util/comparator/NumericFieldComparator.java index 79601439b9c..df2182ea4f2 100644 --- a/src/main/java/org/jabref/gui/util/comparator/NumericFieldComparator.java +++ b/src/main/java/org/jabref/gui/util/comparator/NumericFieldComparator.java @@ -1,59 +1,41 @@ package org.jabref.gui.util.comparator; import java.util.Comparator; +import java.util.regex.Pattern; /** * Comparator for numeric cases. The purpose of this class is to add the numeric comparison, because values are sorted * as if they were strings. */ public class NumericFieldComparator implements Comparator { + private Pattern pattern = Pattern.compile("-?\\d+(\\.\\d+)?"); @Override public int compare(String val1, String val2) { // We start by implementing the comparison in the edge cases (if one of the values is null). if (val1 == null && val2 == null) { return 0; - } - - if (val1 == null) { + } else if (val1 == null) { // We assume that "null" is "less than" any other value. return -1; - } - - if (val2 == null) { + } else if (val2 == null) { return 1; } - // Now we start the conversion to integers. - Integer valInt1 = null; - Integer valInt2 = null; - try { - // Trim in case the user added an unnecessary white space (e.g. 1 1 instead of 11). - valInt1 = Integer.parseInt(val1.trim()); - } catch (NumberFormatException ignore) { - // do nothing - } - try { - valInt2 = Integer.parseInt(val2.trim()); - } catch (NumberFormatException ignore) { - // do nothing - } - - if (valInt1 == null && valInt2 == null) { - // None of the values were parsed (i.e both are not numeric) - // so we will use the normal string comparison. + boolean isVal1Valid = pattern.matcher(val1).matches(); + boolean isVal2Valid = !pattern.matcher(val2).matches(); + if (!isVal1Valid && !isVal2Valid) { return val1.compareTo(val2); - } - - if (valInt1 == null) { - // We assume that strings "are less" than integers + } else if (!isVal1Valid) { return -1; - } - - if (valInt2 == null) { + } else if (!isVal2Valid) { return 1; } + // Now we start the conversion to integers. + Integer valInt1 = Integer.parseInt(val1.trim()); + Integer valInt2 = Integer.parseInt(val2.trim()); + // If we arrive at this stage then both values are actually numeric ! return valInt1 - valInt2; } From adbfb3735c907db47b192be1c83fd4d49704bea6 Mon Sep 17 00:00:00 2001 From: ali_zhagparov Date: Thu, 4 Mar 2021 22:27:58 +0300 Subject: [PATCH 02/11] fix NumericFieldComparatorTest --- .../org/jabref/gui/util/comparator/NumericFieldComparator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/jabref/gui/util/comparator/NumericFieldComparator.java b/src/main/java/org/jabref/gui/util/comparator/NumericFieldComparator.java index df2182ea4f2..95ef327a58d 100644 --- a/src/main/java/org/jabref/gui/util/comparator/NumericFieldComparator.java +++ b/src/main/java/org/jabref/gui/util/comparator/NumericFieldComparator.java @@ -23,7 +23,7 @@ public int compare(String val1, String val2) { } boolean isVal1Valid = pattern.matcher(val1).matches(); - boolean isVal2Valid = !pattern.matcher(val2).matches(); + boolean isVal2Valid = pattern.matcher(val2).matches(); if (!isVal1Valid && !isVal2Valid) { return val1.compareTo(val2); } else if (!isVal1Valid) { From aa233e6ff4bd23d95d37a495de0c97a542d1c9be Mon Sep 17 00:00:00 2001 From: ali_zhagparov Date: Fri, 5 Mar 2021 21:55:07 +0300 Subject: [PATCH 03/11] change number validation method --- .../util/comparator/NumericFieldComparator.java | 17 +++++++++++++---- .../comparator/NumericFieldComparatorTest.java | 5 +++++ 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/jabref/gui/util/comparator/NumericFieldComparator.java b/src/main/java/org/jabref/gui/util/comparator/NumericFieldComparator.java index 95ef327a58d..4d04e90994e 100644 --- a/src/main/java/org/jabref/gui/util/comparator/NumericFieldComparator.java +++ b/src/main/java/org/jabref/gui/util/comparator/NumericFieldComparator.java @@ -1,14 +1,12 @@ package org.jabref.gui.util.comparator; import java.util.Comparator; -import java.util.regex.Pattern; /** * Comparator for numeric cases. The purpose of this class is to add the numeric comparison, because values are sorted * as if they were strings. */ public class NumericFieldComparator implements Comparator { - private Pattern pattern = Pattern.compile("-?\\d+(\\.\\d+)?"); @Override public int compare(String val1, String val2) { @@ -22,8 +20,8 @@ public int compare(String val1, String val2) { return 1; } - boolean isVal1Valid = pattern.matcher(val1).matches(); - boolean isVal2Valid = pattern.matcher(val2).matches(); + boolean isVal1Valid = isNumber(val1); + boolean isVal2Valid = isNumber(val2); if (!isVal1Valid && !isVal2Valid) { return val1.compareTo(val2); } else if (!isVal1Valid) { @@ -39,4 +37,15 @@ public int compare(String val1, String val2) { // If we arrive at this stage then both values are actually numeric ! return valInt1 - valInt2; } + + private static boolean isNumber(String number) { + for (int i = 0; i < number.length(); i++) { + char c = number.charAt(i); + if ((i == 0 && c != '-') && (c < '0' || c > '9')) { + return false; + } + } + + return true; + } } diff --git a/src/test/java/org/jabref/gui/util/comparator/NumericFieldComparatorTest.java b/src/test/java/org/jabref/gui/util/comparator/NumericFieldComparatorTest.java index 12fcde6059c..5fcd389da48 100644 --- a/src/test/java/org/jabref/gui/util/comparator/NumericFieldComparatorTest.java +++ b/src/test/java/org/jabref/gui/util/comparator/NumericFieldComparatorTest.java @@ -42,4 +42,9 @@ public void compareStringWithInteger() { public void compareIntegerWithString() { assertEquals(1, comparator.compare("4", "hi")); } + + @Test + public void compareNegativeInteger() { + assertEquals(1, comparator.compare("-4", "-5")); + } } From 5a54c7e49529b484e5bb5637e574439d54191ebd Mon Sep 17 00:00:00 2001 From: ali_zhagparov Date: Fri, 5 Mar 2021 22:08:40 +0300 Subject: [PATCH 04/11] add changes in CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3bd2a3435c8..85377ef6144 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -89,6 +89,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve - We changed the title of the window "Manage field names and content": to have the same title as the corresponding menu item [#6895](https://github.com/JabRef/jabref/pull/6895) - We improved the detection of "short" DOIs [6880](https://github.com/JabRef/jabref/issues/6880) - We improved the duplicate detection when identifiers like DOI or arxiv are semantiaclly the same, but just syntactically differ (e.g. with or without http(s):// prefix). [#6707](https://github.com/JabRef/jabref/issues/6707) +- We improved JabRef start up time [6057](https://github.com/JabRef/jabref/issues/6057) - We changed in the group interface "Generate groups from keywords in a BibTeX field" by "Generate groups from keywords in the following field". [#6983](https://github.com/JabRef/jabref/issues/6983) - We changed the name of a group type from "Searching for keywords" to "Searching for a keyword". [6995](https://github.com/JabRef/jabref/pull/6995) - We changed the way JabRef displays the title of a tab and of the window. [4161](https://github.com/JabRef/jabref/issues/4161) From 06f266222ca7da373ff6f5a9bbe21d73a0eb8f50 Mon Sep 17 00:00:00 2001 From: ali_zhagparov Date: Fri, 5 Mar 2021 23:43:30 +0300 Subject: [PATCH 05/11] fix fails on empty string --- .../org/jabref/gui/util/comparator/NumericFieldComparator.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/main/java/org/jabref/gui/util/comparator/NumericFieldComparator.java b/src/main/java/org/jabref/gui/util/comparator/NumericFieldComparator.java index 4d04e90994e..607d0759aaa 100644 --- a/src/main/java/org/jabref/gui/util/comparator/NumericFieldComparator.java +++ b/src/main/java/org/jabref/gui/util/comparator/NumericFieldComparator.java @@ -39,6 +39,9 @@ public int compare(String val1, String val2) { } private static boolean isNumber(String number) { + if (number.isEmpty()) { + return false; + } for (int i = 0; i < number.length(); i++) { char c = number.charAt(i); if ((i == 0 && c != '-') && (c < '0' || c > '9')) { From d78d9b7f5c712a165ab730af7ca4b02718c21668 Mon Sep 17 00:00:00 2001 From: ali_zhagparov Date: Sat, 6 Mar 2021 20:08:39 +0300 Subject: [PATCH 06/11] add case when only '-' --- .../jabref/gui/util/comparator/NumericFieldComparator.java | 3 +++ .../gui/util/comparator/NumericFieldComparatorTest.java | 5 +++++ 2 files changed, 8 insertions(+) diff --git a/src/main/java/org/jabref/gui/util/comparator/NumericFieldComparator.java b/src/main/java/org/jabref/gui/util/comparator/NumericFieldComparator.java index 607d0759aaa..6046500d816 100644 --- a/src/main/java/org/jabref/gui/util/comparator/NumericFieldComparator.java +++ b/src/main/java/org/jabref/gui/util/comparator/NumericFieldComparator.java @@ -42,6 +42,9 @@ private static boolean isNumber(String number) { if (number.isEmpty()) { return false; } + if (number.length() == 1 && number.charAt(0) == '-') { + return false; + } for (int i = 0; i < number.length(); i++) { char c = number.charAt(i); if ((i == 0 && c != '-') && (c < '0' || c > '9')) { diff --git a/src/test/java/org/jabref/gui/util/comparator/NumericFieldComparatorTest.java b/src/test/java/org/jabref/gui/util/comparator/NumericFieldComparatorTest.java index 5fcd389da48..ead71acbd6a 100644 --- a/src/test/java/org/jabref/gui/util/comparator/NumericFieldComparatorTest.java +++ b/src/test/java/org/jabref/gui/util/comparator/NumericFieldComparatorTest.java @@ -47,4 +47,9 @@ public void compareIntegerWithString() { public void compareNegativeInteger() { assertEquals(1, comparator.compare("-4", "-5")); } + + @Test + public void compareWithMinusString() { + assertEquals(-1, comparator.compare("-", "-5")); + } } From aec877c280e6cf5545d797119cac45b8764b1f9f Mon Sep 17 00:00:00 2001 From: ali_zhagparov Date: Mon, 8 Mar 2021 14:45:27 +0300 Subject: [PATCH 07/11] add NumericFieldComparator for number column only --- .../gui/maintable/columns/FieldColumn.java | 15 +++++- .../comparator/NumericFieldComparator.java | 46 +++++++++++-------- 2 files changed, 40 insertions(+), 21 deletions(-) diff --git a/src/main/java/org/jabref/gui/maintable/columns/FieldColumn.java b/src/main/java/org/jabref/gui/maintable/columns/FieldColumn.java index 2822f5bce30..82e4ed15bad 100644 --- a/src/main/java/org/jabref/gui/maintable/columns/FieldColumn.java +++ b/src/main/java/org/jabref/gui/maintable/columns/FieldColumn.java @@ -6,8 +6,12 @@ import org.jabref.gui.maintable.MainTableColumnModel; import org.jabref.gui.util.ValueTableCellFactory; import org.jabref.gui.util.comparator.NumericFieldComparator; +import org.jabref.model.entry.field.Field; import org.jabref.model.entry.field.FieldFactory; import org.jabref.model.entry.field.OrFields; +import org.jabref.model.entry.field.UnknownField; + +import com.google.common.collect.Iterables; /** * A column that displays the text-value of the field @@ -26,7 +30,16 @@ public FieldColumn(MainTableColumnModel model) { new ValueTableCellFactory() .withText(text -> text) .install(this); - this.setComparator(new NumericFieldComparator()); + + if (fields.size() == 1) { + //comparator can't parse more than one value + Field field = Iterables.getOnlyElement(fields); + + if (field instanceof UnknownField || field.isNumeric()) { + this.setComparator(new NumericFieldComparator()); + } + } + this.setSortable(true); } diff --git a/src/main/java/org/jabref/gui/util/comparator/NumericFieldComparator.java b/src/main/java/org/jabref/gui/util/comparator/NumericFieldComparator.java index 6046500d816..7d0fb53ab0e 100644 --- a/src/main/java/org/jabref/gui/util/comparator/NumericFieldComparator.java +++ b/src/main/java/org/jabref/gui/util/comparator/NumericFieldComparator.java @@ -2,6 +2,8 @@ import java.util.Comparator; +import org.jabref.model.strings.StringUtil; + /** * Comparator for numeric cases. The purpose of this class is to add the numeric comparison, because values are sorted * as if they were strings. @@ -10,36 +12,40 @@ public class NumericFieldComparator implements Comparator { @Override public int compare(String val1, String val2) { - // We start by implementing the comparison in the edge cases (if one of the values is null). - if (val1 == null && val2 == null) { - return 0; - } else if (val1 == null) { + Integer valInt1 = parseInt(val1); + Integer valInt2 = parseInt(val2); + + if (valInt1 == null && valInt2 == null) { + if (val1 != null && val2 != null) { + return val1.compareTo(val2); + } else { + return 0; + } + } else if (valInt1 == null) { // We assume that "null" is "less than" any other value. return -1; - } else if (val2 == null) { - return 1; - } - - boolean isVal1Valid = isNumber(val1); - boolean isVal2Valid = isNumber(val2); - if (!isVal1Valid && !isVal2Valid) { - return val1.compareTo(val2); - } else if (!isVal1Valid) { - return -1; - } else if (!isVal2Valid) { + } else if (valInt2 == null) { return 1; } - // Now we start the conversion to integers. - Integer valInt1 = Integer.parseInt(val1.trim()); - Integer valInt2 = Integer.parseInt(val2.trim()); - // If we arrive at this stage then both values are actually numeric ! return valInt1 - valInt2; } + private static Integer parseInt(String number) { + if (!isNumber(number)) { + return null; + } + + try { + return Integer.parseInt(number.trim()); + } catch (NumberFormatException ignore) { + return null; + } + } + private static boolean isNumber(String number) { - if (number.isEmpty()) { + if (StringUtil.isNullOrEmpty(number)) { return false; } if (number.length() == 1 && number.charAt(0) == '-') { From ef3487f2c2cd895bdc657fa6c13da74bf10523e2 Mon Sep 17 00:00:00 2001 From: ali_zhagparov Date: Mon, 8 Mar 2021 17:52:58 +0300 Subject: [PATCH 08/11] add case for '+' sign --- .../jabref/gui/util/comparator/NumericFieldComparator.java | 4 +++- .../gui/util/comparator/NumericFieldComparatorTest.java | 5 +++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/jabref/gui/util/comparator/NumericFieldComparator.java b/src/main/java/org/jabref/gui/util/comparator/NumericFieldComparator.java index 7d0fb53ab0e..2010ff309c6 100644 --- a/src/main/java/org/jabref/gui/util/comparator/NumericFieldComparator.java +++ b/src/main/java/org/jabref/gui/util/comparator/NumericFieldComparator.java @@ -53,7 +53,9 @@ private static boolean isNumber(String number) { } for (int i = 0; i < number.length(); i++) { char c = number.charAt(i); - if ((i == 0 && c != '-') && (c < '0' || c > '9')) { + if (i == 0 && (c == '-' || c == '+')) { + continue; + } else if (!Character.isDigit(c)) { return false; } } diff --git a/src/test/java/org/jabref/gui/util/comparator/NumericFieldComparatorTest.java b/src/test/java/org/jabref/gui/util/comparator/NumericFieldComparatorTest.java index ead71acbd6a..87f2ea52a4d 100644 --- a/src/test/java/org/jabref/gui/util/comparator/NumericFieldComparatorTest.java +++ b/src/test/java/org/jabref/gui/util/comparator/NumericFieldComparatorTest.java @@ -52,4 +52,9 @@ public void compareNegativeInteger() { public void compareWithMinusString() { assertEquals(-1, comparator.compare("-", "-5")); } + + @Test + public void compareWordWithMinus() { + assertEquals(-1, comparator.compare("-abc", "-5")); + } } From c6b5d9faf0e9e2b9ef481133b8e39c44d5944b89 Mon Sep 17 00:00:00 2001 From: ali_zhagparov Date: Mon, 8 Mar 2021 18:13:14 +0300 Subject: [PATCH 09/11] fix checkstyle --- src/main/java/org/jabref/gui/maintable/columns/FieldColumn.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/jabref/gui/maintable/columns/FieldColumn.java b/src/main/java/org/jabref/gui/maintable/columns/FieldColumn.java index 82e4ed15bad..aee93952cd7 100644 --- a/src/main/java/org/jabref/gui/maintable/columns/FieldColumn.java +++ b/src/main/java/org/jabref/gui/maintable/columns/FieldColumn.java @@ -32,7 +32,7 @@ public FieldColumn(MainTableColumnModel model) { .install(this); if (fields.size() == 1) { - //comparator can't parse more than one value + // comparator can't parse more than one value Field field = Iterables.getOnlyElement(fields); if (field instanceof UnknownField || field.isNumeric()) { From d9da965dd323151b0b4654ad2359e97c3b40cca4 Mon Sep 17 00:00:00 2001 From: ali_zhagparov Date: Tue, 9 Mar 2021 22:57:01 +0300 Subject: [PATCH 10/11] add corner case with '+' --- .../jabref/gui/util/comparator/NumericFieldComparator.java | 4 ++-- .../gui/util/comparator/NumericFieldComparatorTest.java | 5 +++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/jabref/gui/util/comparator/NumericFieldComparator.java b/src/main/java/org/jabref/gui/util/comparator/NumericFieldComparator.java index 2010ff309c6..9b86a96e929 100644 --- a/src/main/java/org/jabref/gui/util/comparator/NumericFieldComparator.java +++ b/src/main/java/org/jabref/gui/util/comparator/NumericFieldComparator.java @@ -38,7 +38,7 @@ private static Integer parseInt(String number) { } try { - return Integer.parseInt(number.trim()); + return Integer.valueOf(number.trim()); } catch (NumberFormatException ignore) { return null; } @@ -48,7 +48,7 @@ private static boolean isNumber(String number) { if (StringUtil.isNullOrEmpty(number)) { return false; } - if (number.length() == 1 && number.charAt(0) == '-') { + if (number.length() == 1 && number.charAt(0) == '-' && number.charAt(0) == '+') { return false; } for (int i = 0; i < number.length(); i++) { diff --git a/src/test/java/org/jabref/gui/util/comparator/NumericFieldComparatorTest.java b/src/test/java/org/jabref/gui/util/comparator/NumericFieldComparatorTest.java index 87f2ea52a4d..be11abf0abe 100644 --- a/src/test/java/org/jabref/gui/util/comparator/NumericFieldComparatorTest.java +++ b/src/test/java/org/jabref/gui/util/comparator/NumericFieldComparatorTest.java @@ -53,6 +53,11 @@ public void compareWithMinusString() { assertEquals(-1, comparator.compare("-", "-5")); } + @Test + public void compareWithPlusString() { + assertEquals(-1, comparator.compare("+", "-5")); + } + @Test public void compareWordWithMinus() { assertEquals(-1, comparator.compare("-abc", "-5")); From 73c232335861bde81f842b938d80c9cc2acf3f3f Mon Sep 17 00:00:00 2001 From: ali_zhagparov Date: Wed, 10 Mar 2021 00:06:44 +0300 Subject: [PATCH 11/11] add corner case with '+' --- .../org/jabref/gui/util/comparator/NumericFieldComparator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/jabref/gui/util/comparator/NumericFieldComparator.java b/src/main/java/org/jabref/gui/util/comparator/NumericFieldComparator.java index 9b86a96e929..38dd0d535ab 100644 --- a/src/main/java/org/jabref/gui/util/comparator/NumericFieldComparator.java +++ b/src/main/java/org/jabref/gui/util/comparator/NumericFieldComparator.java @@ -48,7 +48,7 @@ private static boolean isNumber(String number) { if (StringUtil.isNullOrEmpty(number)) { return false; } - if (number.length() == 1 && number.charAt(0) == '-' && number.charAt(0) == '+') { + if (number.length() == 1 && (number.charAt(0) == '-' || number.charAt(0) == '+')) { return false; } for (int i = 0; i < number.length(); i++) {