From ef9b7f9dc4d9f0513b36e8630443f099280636c2 Mon Sep 17 00:00:00 2001 From: Michael Brand Date: Fri, 28 Dec 2012 15:02:01 +0100 Subject: [PATCH 5/6] org-table.el: Fix range len bugs and inconsistencies * lisp/org-table.el (org-table-eval-formula): Keep empty fields during preprocessing. (org-table-make-reference): A range with only empty fields should not always return 0 but also empty string, consistent with field reference of an empty field. Use future design for nan but replicate current behavior. * testing/lisp/test-org-table.el: Adapt expected for several ert-deftest. The range len bugs may lead to wrong calculations for range references with empty fields when the range len is relevant. Affects typically Calc vmean on simple range and without format specifier EN. Also Lisp with e. g. `length' on simple range or with L. --- lisp/org-table.el | 13 ++++++++++--- testing/lisp/test-org-table.el | 30 ++++++++---------------------- 2 files changed, 18 insertions(+), 25 deletions(-) diff --git a/lisp/org-table.el b/lisp/org-table.el index 6a9d0b4..1f540b4 100644 --- a/lisp/org-table.el +++ b/lisp/org-table.el @@ -2557,7 +2557,10 @@ not overwrite the stored one." fields))) (if (eq numbers t) (setq fields (mapcar - (lambda (x) (number-to-string (string-to-number x))) + (lambda (x) + (if (string-match "\\S-" x) + (number-to-string (string-to-number x)) + x)) fields))) (setq ndown (1- ndown)) (setq form (copy-sequence formula) @@ -2862,7 +2865,7 @@ LISPP means to return something appropriate for a Lisp list." (delq nil (mapcar (lambda (x) (if (string-match "\\S-" x) x nil)) elements)))) - (setq elements (or elements '("0"))) + (setq elements (or elements '(""))) (if lispp (mapconcat (lambda (x) @@ -2872,7 +2875,11 @@ LISPP means to return something appropriate for a Lisp list." elements " ") (concat "[" (mapconcat (lambda (x) - (if numbers (number-to-string (string-to-number x)) x)) + (if (string-match "\\S-" x) + (if numbers + (number-to-string (string-to-number x)) + x) + (if (or (not keep-empty) numbers) "0" ""))) elements ",") "]")))) diff --git a/testing/lisp/test-org-table.el b/testing/lisp/test-org-table.el index 68949d9..6133005 100644 --- a/testing/lisp/test-org-table.el +++ b/testing/lisp/test-org-table.el @@ -309,12 +309,11 @@ reference (with row). Format specifier L." (org-test-table-target-expect references/target-normal ;; All the #ERROR show that for Lisp calculations N has to be used. - ;; TODO: Len for range reference with only empty fields should be 0. " | 0 | 1 | 0 | 1 | 1 | 1 | 2 | 2 | | z | 1 | z | #ERROR | #ERROR | #ERROR | 2 | 2 | | | 1 | | 1 | 1 | 1 | 1 | 1 | -| | | | 0 | 0 | 0 | 1 | 1 | +| | | | 0 | 0 | 0 | 0 | 0 | " 1 (concat "#+TBLFM: $3 = '(identity \"$1\"); L :: $4 = '(+ $1 $2); L :: " @@ -378,13 +377,11 @@ reference (with row). Format specifier N." "$7 = vlen($1..$2); N :: $8 = vlen(@address@hidden); N"))) (org-test-table-target-expect references/target-normal - ;; TODO: Len for simple range reference with empty field should - ;; also be 1 " | 0 | 1 | 0 | 1 | 1 | 1 | 2 | 2 | | z | 1 | 0 | 1 | 1 | 1 | 2 | 2 | -| | 1 | 0 | 1 | 1 | 1 | 2 | 1 | -| | | 0 | 0 | 0 | 0 | 2 | 1 | +| | 1 | 0 | 1 | 1 | 1 | 1 | 1 | +| | | 0 | 0 | 0 | 0 | 1 | 1 | " 1 lisp calc) (org-test-table-target-expect @@ -453,22 +450,15 @@ reference (with row). Format specifier N." ;; Empty fields in simple and complex range reference: Suppress them ;; ($5 and $6) or keep them and use 0 ($7 and $8) - ;; Calc formula (org-test-table-target-expect "\n| | | 5 | 7 | replace | replace | replace | replace |\n" "\n| | | 5 | 7 | 6 | 6 | 3 | 3 |\n" 1 + ;; Calc formula (concat "#+TBLFM: " "$5 = vmean($1..$4) :: $6 = vmean(@address@hidden) :: " - "$7 = vmean($1..$4); EN :: $8 = vmean(@address@hidden); EN")) - - ;; Lisp formula - ;; TODO: Len for simple range reference with empty field should also - ;; be 6 - (org-test-table-target-expect - "\n| | | 5 | 7 | replace | replace | replace | replace |\n" - "\n| | | 5 | 7 | 3 | 6 | 3 | 3 |\n" - 1 + "$7 = vmean($1..$4); EN :: $8 = vmean(@address@hidden); EN") + ;; Lisp formula (concat "#+TBLFM: " "$5 = '(/ (+ $1..$4 ) (length '( $1..$4 ))); N :: " "$6 = '(/ (+ @address@hidden) (length '(@address@hidden))); N :: " @@ -553,9 +543,7 @@ reference (with row). Format specifier N." (should (equal "0 1" (f '("0" "1") nil nil 'literal))) (should (equal "z 1" (f '("z" "1") nil nil 'literal))) (should (equal "1" (f '("" "1") nil nil 'literal))) - ;; TODO: Should result in empty string like with field reference of - ;; empty field. - (should (equal "0" (f '("" "" ) nil nil 'literal)))) + (should (equal "" (f '("" "" ) nil nil 'literal)))) (ert-deftest test-org-table/org-table-make-reference/format-specifier-none () (fset 'f 'org-table-make-reference) @@ -566,9 +554,7 @@ reference (with row). Format specifier N." (should (equal "\"0\" \"1\"" (f '("0" "1") nil nil t))) (should (equal "\"z\" \"1\"" (f '("z" "1") nil nil t))) (should (equal "\"1\"" (f '("" "1") nil nil t))) - ;; TODO: Should result in empty string like with field reference of - ;; empty field. - (should (equal "\"0\"" (f '("" "" ) nil nil t))) + (should (equal "\"\"" (f '("" "" ) nil nil t))) ;; For Calc formula (should (equal "(0)" (f "0" nil nil nil))) (should (equal "(z)" (f "z" nil nil nil))) -- 1.7.1