From 3a2ac526a61633ada2f3864eb196d11845d3a3d0 Mon Sep 17 00:00:00 2001 From: Christopher Lam Date: Sat, 24 Jan 2026 23:25:42 +0800 Subject: [PATCH 1/6] [gnc-commodity.cpp] improve gnc_commodity_compare ensure it is a stable sort --- libgnucash/engine/gnc-commodity.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libgnucash/engine/gnc-commodity.cpp b/libgnucash/engine/gnc-commodity.cpp index c803a94a9f..91d8867db7 100644 --- a/libgnucash/engine/gnc-commodity.cpp +++ b/libgnucash/engine/gnc-commodity.cpp @@ -1502,13 +1502,13 @@ gnc_commodity_equal(const gnc_commodity * a, const gnc_commodity * b) return gnc_commodity_compare(a, b) == 0; } -// Used as a sorting callback for deleting old prices, so it needs to be -// stable but doesn't need to be in any particular order sensible to humans. int gnc_commodity_compare(const gnc_commodity * a, const gnc_commodity * b) { if (a == b) return 0; if (a && !b) return 1; if (b && !a) return -1; + if (auto rv = g_strcmp0 (gnc_commodity_get_unique_name (a), gnc_commodity_get_unique_name (b))) + return rv; return qof_instance_guid_compare(a, b); } From 5f5caf49b927311986c66e1616c1bd08f6b643b6 Mon Sep 17 00:00:00 2001 From: Christopher Lam Date: Sun, 25 Jan 2026 09:42:19 +0800 Subject: [PATCH 2/6] [trep-engine.scm] use :grid-cell record type --- gnucash/report/trep-engine.scm | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/gnucash/report/trep-engine.scm b/gnucash/report/trep-engine.scm index 7e2e91c2b1..c33515f456 100644 --- a/gnucash/report/trep-engine.scm +++ b/gnucash/report/trep-engine.scm @@ -2068,12 +2068,19 @@ be excluded from periodic reporting.") calculated-cells total-collectors))))) (values table grid csvlist)))) +(define-record-type :grid-cell + (make-grid-cell row col datum) + grid-cell? + (row get-grid-row) + (col get-grid-col) + (datum get-grid-datum)) + ;; grid data structure (define (make-grid) '()) (define (cell-match? cell row col) - (and (or (not row) (equal? row (vector-ref cell 0))) - (or (not col) (equal? col (vector-ref cell 1))))) + (and (or (not row) (equal? row (get-grid-row cell))) + (or (not col) (equal? col (get-grid-col cell))))) (define (grid-get grid row col) ;; grid filter - get all row/col - if #f then retrieve whole row/col (filter @@ -2081,13 +2088,13 @@ be excluded from periodic reporting.") (cell-match? cell row col)) grid)) (define (grid-rows grid) - (delete-duplicates (map (lambda (cell) (vector-ref cell 0)) grid))) + (delete-duplicates (map get-grid-row grid))) (define (grid-cols grid) - (delete-duplicates (map (lambda (cell) (vector-ref cell 1)) grid))) + (delete-duplicates (map get-grid-col grid))) (define (grid-add grid row col data) ;; we don't need to check for duplicate cells in a row/col because ;; in the trep it should never happen. - (cons (vector row col data) grid)) + (cons (make-grid-cell row col data) grid)) (define (grid->html-table grid) (define ( Date: Sun, 25 Jan 2026 09:43:23 +0800 Subject: [PATCH 3/6] [trep-engine.scm] upgrade grid-add formerly, grid-add was simply adding a new (vector row col data) to the grid. it was assuming there was no existing data in row/col. upgrade so that it will consider the existing grid row/col; if there's existing data it will add the monetary amounts into it. --- gnucash/report/trep-engine.scm | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/gnucash/report/trep-engine.scm b/gnucash/report/trep-engine.scm index c33515f456..4070749e2e 100644 --- a/gnucash/report/trep-engine.scm +++ b/gnucash/report/trep-engine.scm @@ -2092,9 +2092,13 @@ be excluded from periodic reporting.") (define (grid-cols grid) (delete-duplicates (map get-grid-col grid))) (define (grid-add grid row col data) - ;; we don't need to check for duplicate cells in a row/col because - ;; in the trep it should never happen. - (cons (make-grid-cell row col data) grid)) + (let lp ((grid grid) (rv '()) (added? #f)) + (match grid + (() (if added? rv (cons (make-grid-cell row col data) rv))) + (((? (cut cell-match? <> row col) this) . rest) + (let ((coll (apply gnc:monetaries-add (append (get-grid-datum this) data)))) + (lp rest (cons (make-grid-cell row col (coll 'format gnc:make-gnc-monetary #f)) rv) #t))) + ((this . rest) (lp rest (cons this rv) added?))))) (define (grid->html-table grid) (define ( Date: Sat, 24 Jan 2026 22:39:43 +0800 Subject: [PATCH 4/6] [trep-engine.scm] refactor grid renderer for each row with multiple commodities, instead of working on commodities via their indices, work on the commodities directly. without this refactoring, a subtotal table row with multiple commodities may show different commodities on the same line. with this refactoring, the row will show a stable list of commodities. --- gnucash/report/trep-engine.scm | 40 +++++++++++++++++----------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/gnucash/report/trep-engine.scm b/gnucash/report/trep-engine.scm index 4070749e2e..a1815b83bf 100644 --- a/gnucash/report/trep-engine.scm +++ b/gnucash/report/trep-engine.scm @@ -2115,34 +2115,35 @@ be excluded from periodic reporting.") (gnc:make-gnc-monetary currency (gnc-numeric-convert (/ amount divisor) scu GNC-HOW-RND-ROUND))))) - (define (row->num-of-commodities row) - ;; for a row, find the maximum number of commodities being stored - (apply max - (map (lambda (col) - (let ((cell (grid-get grid row col))) - (if (null? cell) 0 - (length (get-grid-datum (car cell)))))) - (cons 'col-total list-of-cols)))) - (define (make-table-cell row col commodity-idx divisor) + (define (row->commodities row) + (sort! (fold (lambda (cell acc) + (fold (lambda (mon acc2) + (let ((comm (gnc:gnc-monetary-commodity mon))) + (if (member comm acc2) acc2 (cons comm acc2)))) + acc (get-grid-datum cell))) + '() (grid-get grid row #f)) + (lambda (a b) (< (gnc-commodity-compare a b) 0)))) + (define (make-table-cell row col commodity divisor) (let ((cell (grid-get grid row col))) (if (null? cell) "" (gnc:make-html-table-cell/markup "number-cell" (monetary-div - (list-ref-safe (get-grid-datum (car cell)) commodity-idx) + (find (lambda (mon) (equal? commodity (gnc:gnc-monetary-commodity mon))) + (get-grid-datum (car cell))) divisor))))) - (define (make-row row commodity-idx) + (define (make-row row commodity first?) (append (list (cond - ((positive? commodity-idx) "") + ((not first?) "") ((eq? row 'row-total) (G_ "Grand Total")) (else (cdr row)))) - (map (lambda (col) (make-table-cell row col commodity-idx 1)) + (map (lambda (col) (make-table-cell row col commodity 1)) list-of-cols) - (list (make-table-cell row 'col-total commodity-idx 1)) + (list (make-table-cell row 'col-total commodity 1)) (if row-average-enabled? (list (make-table-cell - row 'col-total commodity-idx (length list-of-cols))) + row 'col-total commodity (length list-of-cols))) '()))) (let ((table (gnc:make-html-table))) (gnc:html-table-set-caption! table (G_ optname-grid)) @@ -2156,11 +2157,10 @@ be excluded from periodic reporting.") 'attribute (list "class" "column-heading-right")) (for-each (lambda (row) - (for-each - (lambda (commodity-idx) - (gnc:html-table-append-row! - table (make-row row commodity-idx))) - (iota (row->num-of-commodities row)))) + (let lp ((commodities (row->commodities row)) (first? #t)) + (unless (null? commodities) + (gnc:html-table-append-row! table (make-row row (car commodities) first?)) + (lp (cdr commodities) #f)))) (if (memq 'row-total (grid-rows grid)) (append list-of-rows '(row-total)) list-of-rows)) From 9bbe29b42d5068253dcf746e94de2b48f10f0d54 Mon Sep 17 00:00:00 2001 From: Christopher Lam Date: Sat, 24 Jan 2026 21:29:12 +0800 Subject: [PATCH 5/6] [trep-engine.scm] tally secondary subtotals in subtotal table when displaying secondary-level subtotals, also tally the amounts in the row-total grid --- .../reports/standard/test/test-transaction.scm | 14 +++++++------- gnucash/report/trep-engine.scm | 8 +++++--- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/gnucash/report/reports/standard/test/test-transaction.scm b/gnucash/report/reports/standard/test/test-transaction.scm index 646f4b2140..11dbb05f48 100644 --- a/gnucash/report/reports/standard/test/test-transaction.scm +++ b/gnucash/report/reports/standard/test/test-transaction.scm @@ -962,10 +962,10 @@ "-#51.00" "-#51.00" "-#51.00" "-#51.00" "-#51.00" "-#51.00" "-#612.00" "-#51.00") (get-row-col sxml 5 #f)) (test-equal "summary gbp total-row is correct" - (list "Grand Total" "#0.00" "#0.00") + (list "Total" "#0.00" "#0.00" "#0.00" "#0.00" "#0.00" "#0.00" "#0.00" "#0.00" "#0.00" "#0.00" "#0.00" "#0.00" "#0.00" "#0.00") (get-row-col sxml 6 #f)) (test-equal "summary total-row is correct" - (list "$0.00" "$0.00") + (list "$0.00" "$0.00" "$0.00" "$0.00" "$0.00" "$0.00" "$0.00" "$0.00" "$0.00" "$0.00" "$0.00" "$0.00" "$0.00" "$0.00") (get-row-col sxml 7 #f))) (set-option! options "General" "Start Date" (cons 'absolute (gnc-dmy2time64 01 01 1969))) @@ -981,19 +981,19 @@ (list "Income" "-$29.00" "-$29.00" "-$9.67") (get-row-col sxml 3 #f)) (test-equal "sparse summary-table - row 4" - (list "Grand Total" "$3.00" "$1.00") + (list "Total" "$0.00" "$11.00" "-$8.00" "$3.00" "$1.00") (get-row-col sxml 4 #f)) (test-equal "sparse summary-table - col 1" - (list "Bank" "Expenses" "Income" "Grand Total") + (list "Bank" "Expenses" "Income" "Total") (get-row-col sxml #f 1)) (test-equal "sparse summary-table - col 2" - (list "$29.00" "-$29.00") + (list "$29.00" "-$29.00" "$0.00") (get-row-col sxml #f 2)) (test-equal "sparse summary-table - col 3" - (list "-$5.00" "$16.00") + (list "-$5.00" "$16.00" "$11.00") (get-row-col sxml #f 3)) (test-equal "sparse summary-table - col 4" - (list "-$23.00" "$15.00") + (list "-$23.00" "$15.00" "-$8.00") (get-row-col sxml #f 4)) (test-equal "sparse summary-table - col 5" (list "$1.00" "$31.00" "-$29.00" "$3.00") diff --git a/gnucash/report/trep-engine.scm b/gnucash/report/trep-engine.scm index a1815b83bf..36a55b0b9e 100644 --- a/gnucash/report/trep-engine.scm +++ b/gnucash/report/trep-engine.scm @@ -1780,8 +1780,10 @@ be excluded from periodic reporting.") (or (and first-column-merge? (retrieve-commodity (cadr columns) commodity)) zero)))) - (set! grid - (grid-add grid row col (map get-commodity-grid-amount list-of-commodities))) + (let ((amounts (map get-commodity-grid-amount list-of-commodities))) + (set! grid (grid-add grid row col amounts)) + (when (eq? level 'secondary) + (set! grid (grid-add grid 'row-total col amounts)))) ;; each commodity subtotal gets a separate line in the html-table ;; each line comprises: indenting, first-column, data-columns @@ -2136,7 +2138,7 @@ be excluded from periodic reporting.") (append (list (cond ((not first?) "") - ((eq? row 'row-total) (G_ "Grand Total")) + ((eq? row 'row-total) (G_ "Total")) (else (cdr row)))) (map (lambda (col) (make-table-cell row col commodity 1)) list-of-cols) From 9d07ed7726580bc37327364235cbafe4d6c22ce0 Mon Sep 17 00:00:00 2001 From: Christopher Lam Date: Sun, 25 Jan 2026 10:26:10 +0800 Subject: [PATCH 6/6] [trep-engine] Encapsulate grid state behind a closure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of passing the grid list into grid-* procedures, `make-grid` now returns a dispatcher that owns the grid and implements operations selected by an argument. • Removes repeated `grid` parameters from helpers • Makes row/column/HTML logic operate on shared private state • Provides a clearer API: 'add 'get-html --- gnucash/report/trep-engine.scm | 221 ++++++++++++++++++--------------- 1 file changed, 122 insertions(+), 99 deletions(-) diff --git a/gnucash/report/trep-engine.scm b/gnucash/report/trep-engine.scm index 36a55b0b9e..5858a0f45a 100644 --- a/gnucash/report/trep-engine.scm +++ b/gnucash/report/trep-engine.scm @@ -1781,9 +1781,9 @@ be excluded from periodic reporting.") zero)))) (let ((amounts (map get-commodity-grid-amount list-of-commodities))) - (set! grid (grid-add grid row col amounts)) + (grid 'add row col amounts) (when (eq? level 'secondary) - (set! grid (grid-add grid 'row-total col amounts)))) + (grid 'add 'row-total col amounts))) ;; each commodity subtotal gets a separate line in the html-table ;; each line comprises: indenting, first-column, data-columns @@ -2070,103 +2070,126 @@ be excluded from periodic reporting.") calculated-cells total-collectors))))) (values table grid csvlist)))) -(define-record-type :grid-cell - (make-grid-cell row col datum) - grid-cell? - (row get-grid-row) - (col get-grid-col) - (datum get-grid-datum)) - -;; grid data structure (define (make-grid) - '()) -(define (cell-match? cell row col) - (and (or (not row) (equal? row (get-grid-row cell))) - (or (not col) (equal? col (get-grid-col cell))))) -(define (grid-get grid row col) - ;; grid filter - get all row/col - if #f then retrieve whole row/col - (filter - (lambda (cell) - (cell-match? cell row col)) - grid)) -(define (grid-rows grid) - (delete-duplicates (map get-grid-row grid))) -(define (grid-cols grid) - (delete-duplicates (map get-grid-col grid))) -(define (grid-add grid row col data) - (let lp ((grid grid) (rv '()) (added? #f)) - (match grid - (() (if added? rv (cons (make-grid-cell row col data) rv))) - (((? (cut cell-match? <> row col) this) . rest) - (let ((coll (apply gnc:monetaries-add (append (get-grid-datum this) data)))) - (lp rest (cons (make-grid-cell row col (coll 'format gnc:make-gnc-monetary #f)) rv) #t))) - ((this . rest) (lp rest (cons this rv) added?))))) -(define (grid->html-table grid) - (define (commodities row) - (sort! (fold (lambda (cell acc) - (fold (lambda (mon acc2) - (let ((comm (gnc:gnc-monetary-commodity mon))) - (if (member comm acc2) acc2 (cons comm acc2)))) - acc (get-grid-datum cell))) - '() (grid-get grid row #f)) - (lambda (a b) (< (gnc-commodity-compare a b) 0)))) - (define (make-table-cell row col commodity divisor) - (let ((cell (grid-get grid row col))) - (if (null? cell) "" - (gnc:make-html-table-cell/markup - "number-cell" - (monetary-div - (find (lambda (mon) (equal? commodity (gnc:gnc-monetary-commodity mon))) - (get-grid-datum (car cell))) - divisor))))) - (define (make-row row commodity first?) - (append - (list (cond - ((not first?) "") - ((eq? row 'row-total) (G_ "Total")) - (else (cdr row)))) - (map (lambda (col) (make-table-cell row col commodity 1)) - list-of-cols) - (list (make-table-cell row 'col-total commodity 1)) - (if row-average-enabled? - (list (make-table-cell - row 'col-total commodity (length list-of-cols))) - '()))) - (let ((table (gnc:make-html-table))) - (gnc:html-table-set-caption! table (G_ optname-grid)) - (gnc:html-table-set-col-headers! - table (append (list "") - (map cdr list-of-cols) - (list (G_ "Total")) - (if row-average-enabled? (list (G_ "Average")) '()))) - (gnc:html-table-set-style! - table "th" - 'attribute (list "class" "column-heading-right")) - (for-each - (lambda (row) - (let lp ((commodities (row->commodities row)) (first? #t)) - (unless (null? commodities) - (gnc:html-table-append-row! table (make-row row (car commodities) first?)) - (lp (cdr commodities) #f)))) - (if (memq 'row-total (grid-rows grid)) - (append list-of-rows '(row-total)) - list-of-rows)) - table)) + + (define-record-type :grid-cell + (make-grid-cell row col datum) + grid-cell? + (row get-grid-row) + (col get-grid-col) + (datum get-grid-datum)) + + (let ((cells '())) + + (define (cell-match? cell row col) + (and (or (not row) (equal? row (get-grid-row cell))) + (or (not col) (equal? col (get-grid-col cell))))) + + (define (grid-get row col) + (filter (cut cell-match? <> row col) cells)) + + (define (grid-rows) + (delete-duplicates (map get-grid-row cells))) + + (define (grid-cols) + (delete-duplicates (map get-grid-col cells))) + + (define (grid-add row col data) + (let lp ((rest cells) (rv '()) (added? #f)) + (match rest + (() (set! cells (if added? rv (cons (make-grid-cell row col data) rv)))) + (((? (cut cell-match? <> row col) this) . more) + (let* ((coll (apply gnc:monetaries-add (append (get-grid-datum this) data))) + (new-cell (make-grid-cell row col (coll 'format gnc:make-gnc-monetary #f)))) + (lp more (cons new-cell rv) #t))) + ((this . more) (lp more (cons this rv) added?))))) + + (define (row->commodities row) + (sort! + (fold (lambda (cell acc) + (fold (lambda (mon acc2) + (let ((comm (gnc:gnc-monetary-commodity mon))) + (if (member comm acc2) acc2 (cons comm acc2)))) + acc + (get-grid-datum cell))) + '() + (grid-get row #f)) + (lambda (a b) (< (gnc-commodity-compare a b) 0)))) + + (define (grid->html-table) + (define ( commodity 1) list-of-cols) + (list (make-table-cell row 'col-total commodity 1)) + (if row-average-enabled? + (list (make-table-cell row 'col-total commodity (length list-of-cols))) + '()))) + + (let ((table (gnc:make-html-table))) + (gnc:html-table-set-caption! table (G_ optname-grid)) + (gnc:html-table-set-col-headers! + table (append (list "") + (map cdr list-of-cols) + (list (G_ "Total")) + (if row-average-enabled? (list (G_ "Average")) '()))) + (gnc:html-table-set-style! + table "th" + 'attribute (list "class" "column-heading-right")) + + (for-each + (lambda (row) + (let lp ((commodities (row->commodities row)) (first? #t)) + (unless (null? commodities) + (gnc:html-table-append-row! + table (make-row row (car commodities) first?)) + (lp (cdr commodities) #f)))) + (if (memq 'row-total (grid-rows)) + (append list-of-rows '(row-total)) + list-of-rows)) + table)) + + (lambda (msg . args) + (case msg + ((add) (apply grid-add args)) + ;; ((get) (apply grid-get args)) ;; (grid 'get row col) + ;; ((rows) (grid-rows)) + ;; ((cols) (grid-cols)) + ;; ((clear) (set! cells '())) + ((get-html) (grid->html-table)) + (else (error "Unknown grid operation" msg)))))) (define* (gnc:trep-renderer report-obj #:key custom-calculated-cells empty-report-message @@ -2628,7 +2651,7 @@ be excluded from periodic reporting.") (gnc:html-render-options-changed options))) (when subtotal-table? - (gnc:html-document-add-object! document (grid->html-table grid))) + (gnc:html-document-add-object! document (grid 'get-html))) (unless (and subtotal-table? (opt-val pagename-sorting optname-show-subtotals-only))