gnucash maint: Improve transaction sorting on effective num field.
John Ralls
jralls at code.gnucash.org
Tue Jun 15 17:03:06 EDT 2021
Updated via https://github.com/Gnucash/gnucash/commit/601eb513 (commit)
from https://github.com/Gnucash/gnucash/commit/84fb25a0 (commit)
commit 601eb513619a9d8d9a609ac22d5e3b20527ed6ad
Author: John Ralls <jralls at ceridwen.us>
Date: Tue Jun 8 09:49:46 2021 -0700
Improve transaction sorting on effective num field.
Inspired by PR #983. Transaction sorting on num broke down if the
user had a non-numeric string or a number larger than an int in
the effective num field (might be split-action if the option is set).
The comparison first tries to use strtoull on the two strings and
compares the results. If they're both nonzero and different then the
numeric order is returned. If they're both nonzero but the same the
unconverted parts of each are passed to g_utf8_collate; if either is 0
then the whole strings are passed to g_utf8_collate. strtoull will
return 0 for a negative number.
diff --git a/libgnucash/engine/Transaction.c b/libgnucash/engine/Transaction.c
index 2865b237e..e7c3cba0a 100644
--- a/libgnucash/engine/Transaction.c
+++ b/libgnucash/engine/Transaction.c
@@ -1881,12 +1881,43 @@ xaccTransOrder (const Transaction *ta, const Transaction *tb)
return xaccTransOrder_num_action (ta, NULL, tb, NULL);
}
+/* Order a pair of potentially numeric string as numbers if both
+ * strings begin with numbers, ordering the remainder of the string
+ * lexically if the numeric parts are equal, and the whole strings
+ * lexically otherwise.
+ *
+ * Note that this won't work well for numbers > 10^18 and that
+ * negative numbers are treated as strings and will cause the pair to
+ * be ordered lexically.
+ */
+
+static int
+order_by_int64_or_string (const char* a, const char* b)
+{
+ char *end_a = NULL, *end_b = NULL;
+ int cmp = 0;
+ uint64_t na = strtoull(a, &end_a, 10);
+ uint64_t nb = strtoull(b, &end_b, 10);
+ if (na && nb)
+ {
+ if (na != nb)
+ return na < nb ? -1 : 1;
+ cmp = g_utf8_collate(end_a, end_b);
+ }
+ else
+ {
+ cmp = g_utf8_collate(a, b);
+ }
+ return cmp < 0 ? -1 : cmp > 0 ? 1 : 0;
+}
+
int
xaccTransOrder_num_action (const Transaction *ta, const char *actna,
const Transaction *tb, const char *actnb)
{
char *da, *db;
- int na, nb, retval;
+ int retval;
+ int64_t na, nb;
if ( ta && !tb ) return -1;
if ( !ta && tb ) return +1;
@@ -1906,16 +1937,14 @@ xaccTransOrder_num_action (const Transaction *ta, const char *actna,
/* otherwise, sort on number string */
if (actna && actnb) /* split action string, if not NULL */
{
- na = atoi(actna);
- nb = atoi(actnb);
+ retval = order_by_int64_or_string (actna, actnb);
}
else /* else transaction num string */
{
- na = atoi(ta->num);
- nb = atoi(tb->num);
+ retval = order_by_int64_or_string (ta->num, tb->num);
}
- if (na < nb) return -1;
- if (na > nb) return +1;
+ if (retval)
+ return retval;
if (ta->date_entered != tb->date_entered)
return (ta->date_entered > tb->date_entered) - (ta->date_entered < tb->date_entered);
diff --git a/libgnucash/engine/test/utest-Transaction.cpp b/libgnucash/engine/test/utest-Transaction.cpp
index ec8de4c4b..3cf10ebb8 100644
--- a/libgnucash/engine/test/utest-Transaction.cpp
+++ b/libgnucash/engine/test/utest-Transaction.cpp
@@ -1763,8 +1763,14 @@ test_xaccTransOrder_num_action (Fixture *fixture, gconstpointer pData)
g_assert_cmpint (xaccTransOrder_num_action (txnA, NULL, txnB, NULL), ==, -1);
txnB->num = static_cast<char*>(CACHE_INSERT ("101"));
g_assert_cmpint (xaccTransOrder_num_action (txnA, NULL, txnB, NULL), ==, 1);
- txnB->num = static_cast<char*>(CACHE_INSERT ("one-oh-one"));
+ txnA->num = static_cast<char*>(CACHE_INSERT ("12a"));
+ g_assert_cmpint (xaccTransOrder_num_action (txnA, NULL, txnB, NULL), ==, -1);
+ txnB->num = static_cast<char*>(CACHE_INSERT ("12c"));
+ g_assert_cmpint (xaccTransOrder_num_action (txnA, NULL, txnB, NULL), ==, -1);
+ txnB->num = static_cast<char*>(CACHE_INSERT ("12"));
g_assert_cmpint (xaccTransOrder_num_action (txnA, NULL, txnB, NULL), ==, 1);
+ txnB->num = static_cast<char*>(CACHE_INSERT ("one-oh-one"));
+ g_assert_cmpint (xaccTransOrder_num_action (txnA, NULL, txnB, NULL), ==, -1);
g_assert_cmpint (xaccTransOrder_num_action (txnA, "24", txnB, "42"), ==, -1);
txnB->date_posted -= 1;
g_assert_cmpint (xaccTransOrder_num_action (txnA, "24", txnB, "42"), ==, 1);
Summary of changes:
libgnucash/engine/Transaction.c | 43 +++++++++++++++++++++++-----
libgnucash/engine/test/utest-Transaction.cpp | 8 +++++-
2 files changed, 43 insertions(+), 8 deletions(-)
More information about the gnucash-changes
mailing list