Browse Source

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.
Michael Brand 5 years ago
parent
commit
764315b3fc
2 changed files with 18 additions and 25 deletions
  1. 10 3
      lisp/org-table.el
  2. 8 22
      testing/lisp/test-org-table.el

+ 10 - 3
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
 		   ",") "]"))))
 

+ 8 - 22
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(@0$1..@0$2); 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(@0$1..@0$4) :: "
-	   "$7 = vmean($1..$4); EN :: $8 = vmean(@0$1..@0$4); 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(@0$1..@0$4); EN")
+   ;; Lisp formula
    (concat "#+TBLFM: "
 	   "$5 = '(/ (+   $1..$4  ) (length '(  $1..$4  )));  N :: "
 	   "$6 = '(/ (+ @0$1..@0$4) (length '(@0$1..@0$4)));  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)))