emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
From: Michael Brand <michael.ch.brand@gmail.com>
To: Org Mode <emacs-orgmode@gnu.org>
Cc: Nick Dokos <ndokos@gmail.com>, Gunnar Wolf <gwolf@gwolf.org>
Subject: Re: Tables for attendance lists - A problem understanding TBLFM?
Date: Tue, 9 Apr 2013 14:40:06 +0200	[thread overview]
Message-ID: <CALn3zogNtKB2W0x6OG-xNkaKXxZan4Vwk2_4UaCamoR5apHRWg@mail.gmail.com> (raw)
In-Reply-To: <CANU-iVAbsDbrAqo-8dp1OW-UOzJuaWgi-7YRuvxNpAdc+fAP2Q@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 814 bytes --]

On Tue, Apr 9, 2013 at 5:31 AM, Nick Dokos <ndokos@gmail.com> wrote:
> You can turn on formula debugging with C-c { and then you'd
> see that in Pancho's case, the list is ("") i.e. a list containing the
> empty string - a list of length 1. That might qualify as a bug (or not)

This issue is part of some old bugs that I discovered end of 2012. It
seems like my patch from then
http://orgmode.org/w/org-mode.git?p=org-mode.git;a=commitdiff;h=764315
resolved it only partially and I missed the case of a range with only
empty fields, although I tested and approved it in my ERTs... The
attached patch corrects.

It is worth a small compatibility change: For a range with only empty
fields it is now possible and necessary to choose different behaviors
of vmean by adding the format specifiers E and/or N.

Michael

[-- Attachment #2: 0001-org-table.el-Fix-range-len-bugs-for-empty-ranges.patch.txt --]
[-- Type: text/plain, Size: 7756 bytes --]

From 758414846936936f4e985fdb4de82eb7c95f163f Mon Sep 17 00:00:00 2001
From: Michael Brand <michael.ch.brand@gmail.com>
Date: Tue, 9 Apr 2013 14:35:21 +0200
Subject: [PATCH] org-table.el: Fix range len bugs for empty ranges

(org-table-make-reference): A range with only empty fields should lead
to length 0.
* 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.

It is worth a small compatibility change: For a range with only empty
fields it is now possible and necessary to choose different behaviors
of vmean by adding the format specifiers E and/or N.

This is a follow-up of commit
764315b3fce26de59189b957a8049e299209043a.
---
 lisp/org-table.el              |  7 +++--
 testing/lisp/test-org-table.el | 60 ++++++++++++++++++++++++++----------------
 2 files changed, 42 insertions(+), 25 deletions(-)

diff --git a/lisp/org-table.el b/lisp/org-table.el
index bdf4ad8..7122c87 100644
--- a/lisp/org-table.el
+++ b/lisp/org-table.el
@@ -2929,7 +2929,10 @@ list, 'literal is for the format specifier L."
       (if lispp
 	  (if (eq lispp 'literal)
 	      elements
-	    (prin1-to-string (if numbers (string-to-number elements) elements)))
+	    (if (and (eq elements "") (not keep-empty))
+		""
+	      (prin1-to-string
+	       (if numbers (string-to-number elements) elements))))
 	(if (string-match "\\S-" elements)
 	    (progn
 	      (when numbers (setq elements (number-to-string
@@ -2942,7 +2945,7 @@ list, 'literal is for the format specifier L."
 	    (delq nil
 		  (mapcar (lambda (x) (if (string-match "\\S-" x) x nil))
 			  elements))))
-    (setq elements (or elements '("")))
+    (setq elements (or elements '()))  ; if delq returns nil then we need '()
     (if lispp
 	(mapconcat
 	 (lambda (x)
diff --git a/testing/lisp/test-org-table.el b/testing/lisp/test-org-table.el
index 01adf52..2dd5f38 100644
--- a/testing/lisp/test-org-table.el
+++ b/testing/lisp/test-org-table.el
@@ -339,7 +339,7 @@ reference (with row).  No format specifier."
 | 0 | 1 | 0 | #ERROR | #ERROR | #ERROR | 2 | 2 |
 | z | 1 | z | #ERROR | #ERROR | #ERROR | 2 | 2 |
 |   | 1 |   | #ERROR | #ERROR | #ERROR | 1 | 1 |
-|   |   |   | #ERROR | #ERROR | #ERROR | 1 | 1 |
+|   |   |   | #ERROR | 0      | 0      | 0 | 0 |
 "
      1 lisp)
     (org-test-table-target-expect
@@ -348,7 +348,7 @@ reference (with row).  No format specifier."
 | 0 | 1 | 0 |     1 |     1 |     1 | 2 | 2 |
 | z | 1 | z | z + 1 | z + 1 | z + 1 | 2 | 2 |
 |   | 1 | 0 |     1 |     1 |     1 | 1 | 1 |
-|   |   | 0 |     0 |     0 |     0 | 1 | 1 |
+|   |   | 0 |     0 |     0 |     0 | 0 | 0 |
 "
      1 calc)
     (org-test-table-target-expect
@@ -381,7 +381,7 @@ reference (with row).  Format specifier N."
 | 0 | 1 | 0 | 1 | 1 | 1 | 2 | 2 |
 | z | 1 | 0 | 1 | 1 | 1 | 2 | 2 |
 |   | 1 | 0 | 1 | 1 | 1 | 1 | 1 |
-|   |   | 0 | 0 | 0 | 0 | 1 | 1 |
+|   |   | 0 | 0 | 0 | 0 | 0 | 0 |
 "
      1 lisp calc)
     (org-test-table-target-expect
@@ -455,20 +455,34 @@ 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)
 
-  (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
-   (concat "#+TBLFM: "
-	   "$5 = '(/ (+   $1..$4  ) (length '(  $1..$4  )));  N :: "
-	   "$6 = '(/ (+ @0$1..@0$4) (length '(@0$1..@0$4)));  N :: "
-	   "$7 = '(/ (+   $1..$4  ) (length '(  $1..$4  ))); EN :: "
-	   "$8 = '(/ (+ @0$1..@0$4) (length '(@0$1..@0$4))); EN"))
+  (let ((calc (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 (concat
+	       "#+TBLFM: "
+	       "$5 = '(/ (+   $1..$4  ) (length '(  $1..$4  )));  N :: "
+	       "$6 = '(/ (+ @0$1..@0$4) (length '(@0$1..@0$4)));  N :: "
+	       "$7 = '(/ (+   $1..$4  ) (length '(  $1..$4  ))); EN :: "
+	       "$8 = '(/ (+ @0$1..@0$4) (length '(@0$1..@0$4))); EN")))
+    (org-test-table-target-expect
+     "\n|   |   | 5 | 7 | replace | replace | replace | replace |\n"
+     "\n|   |   | 5 | 7 | 6 | 6 | 3 | 3 |\n"
+     1 calc lisp)
+
+    ;; The mean value of a range with only empty fields is not defined
+    (let ((target
+	   "\n|   |   |   |   | replace | replace | replace | replace |\n"))
+      (org-test-table-target-expect
+       target
+       "\n|   |   |   |   | vmean([]) | vmean([]) | 0 | 0 |\n"
+       1 calc)
+      (org-test-table-target-expect
+       target
+       "\n|   |   |   |   | #ERROR | #ERROR | 0 | 0 |\n"
+       1 lisp)))
 
   ;; Test if one field is empty, else do a calculation
   (org-test-table-target-expect
@@ -667,11 +681,11 @@ reference (with row).  Format specifier N."
   ;; For Lisp formula
   (should (equal "\"0\""       (f   "0"         nil nil t)))
   (should (equal "\"z\""       (f   "z"         nil nil t)))
-  (should (equal  "\"\""       (f   ""          nil nil t)))
+  (should (equal   ""          (f   ""          nil nil t)))
   (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)))
-  (should (equal    "\"\""     (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)))
@@ -679,7 +693,7 @@ reference (with row).  Format specifier N."
   (should (equal  "[0,1]"      (f '("0"    "1") nil nil nil)))
   (should (equal  "[z,1]"      (f '("z"    "1") nil nil nil)))
   (should (equal    "[1]"      (f '(""     "1") nil nil nil)))
-  (should (equal   "[0]"       (f '(""     "" ) nil nil nil)))
+  (should (equal   "[]"        (f '(""     "" ) nil nil nil)))
   ;; For Calc formula, special numbers
   (should (equal  "(nan)"      (f    "nan"      nil nil nil)))
   (should (equal "(uinf)"      (f   "uinf"      nil nil nil)))
@@ -695,11 +709,11 @@ reference (with row).  Format specifier N."
   ;; For Lisp formula
   (should (equal  "0"    (f   "0"         nil t t)))
   (should (equal  "0"    (f   "z"         nil t t)))
-  (should (equal  "0"    (f   ""          nil t t)))
+  (should (equal  ""     (f   ""          nil t t)))
   (should (equal  "0 1"  (f '("0"    "1") nil t t)))
   (should (equal  "0 1"  (f '("z"    "1") nil t t)))
   (should (equal    "1"  (f '(""     "1") nil t t)))
-  (should (equal   "0"   (f '(""     "" ) nil t t)))
+  (should (equal   ""    (f '(""     "" ) nil t t)))
   ;; For Calc formula
   (should (equal "(0)"   (f   "0"         nil t nil)))
   (should (equal "(0)"   (f   "z"         nil t nil)))
@@ -707,7 +721,7 @@ reference (with row).  Format specifier N."
   (should (equal "[0,1]" (f '("0"    "1") nil t nil)))
   (should (equal "[0,1]" (f '("z"    "1") nil t nil)))
   (should (equal   "[1]" (f '(""     "1") nil t nil)))
-  (should (equal  "[0]"  (f '(""     "" ) nil t nil)))
+  (should (equal  "[]"   (f '(""     "" ) nil t nil)))
   ;; For Calc formula, special numbers
   (should (equal "(0)"   (f    "nan"      nil t nil)))
   (should (equal "(0)"   (f   "uinf"      nil t nil)))
-- 
1.8.1.2


  parent reply	other threads:[~2013-04-09 12:40 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-08 23:57 Tables for attendance lists - A problem understanding TBLFM? Gunnar Wolf
2013-04-09  0:25 ` Suvayu Ali
2013-04-09  2:21   ` Gunnar Wolf
2013-04-09  3:31     ` Nick Dokos
2013-04-09  3:34       ` Nick Dokos
2013-04-09 12:40       ` Michael Brand [this message]
2013-04-09 14:57         ` Gunnar Wolf
2013-04-09 17:06         ` Bastien
2013-04-11 12:15           ` Michael Brand
2013-04-14  8:04           ` Michael Brand
2013-04-14 10:11             ` Bastien
2013-04-14 11:17               ` Michael Brand
2013-04-14 23:28                 ` Bastien
2013-04-09 14:55       ` Gunnar Wolf
2013-04-09 15:10         ` Nick Dokos
2013-04-09 15:31           ` Gunnar Wolf
2013-04-09 15:34           ` Michael Brand
2013-04-09 15:50             ` Michael Brand
2013-04-09 16:03               ` Nick Dokos
2013-04-09 16:13                 ` Gunnar Wolf
2013-04-09 12:41 ` Michael Brand

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.orgmode.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CALn3zogNtKB2W0x6OG-xNkaKXxZan4Vwk2_4UaCamoR5apHRWg@mail.gmail.com \
    --to=michael.ch.brand@gmail.com \
    --cc=emacs-orgmode@gnu.org \
    --cc=gwolf@gwolf.org \
    --cc=ndokos@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).