Hello,

attached there are a few patches reworking the parsing of org-table
formula mode strings and introducing a new mode flag to enable Calc's
units simplification mode as discussed in a recent thread on the mailing
list. I haven't updated the documentation. I can do it once we agree
that this feature is a good idea.

I have already submitted the patches in another thread
https://orgmode.org/list/6d8c15c2-d1b0-d913-df39-c60381cff...@grinta.net/T/#m03a426dd8476b60019dfffecb8781a2126df690f
but it seems that woof did not pick them up, thus I am re-sending them.

Cheers,
Dan
From c7434974897d932fe3acd182f06a98a61719e208 Mon Sep 17 00:00:00 2001
From: Daniele Nicolodi <dani...@grinta.net>
Date: Tue, 20 Oct 2020 11:03:14 +0200
Subject: [PATCH 1/4] org-table: Fix table formula mode string handling

* lisp/org-table.el (org-table-eval-formula): Move mode lookup table
from org-table--set-calc-mode to here.

* lisp/org-table.el (org-table--set-calc-mode): Use plist-put instead
of the buggy open coded version.
---
 lisp/org-table.el | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/lisp/org-table.el b/lisp/org-table.el
index 112b1e171..0790dc3ca 100644
--- a/lisp/org-table.el
+++ b/lisp/org-table.el
@@ -721,17 +721,8 @@ Field is restored even in case of abnormal exit."
         (org-table-goto-column ,column)
         (set-marker ,line nil)))))
 
-(defsubst org-table--set-calc-mode (var &optional value)
-  (if (stringp var)
-      (setq var (assoc var '(("D" calc-angle-mode deg)
-                            ("R" calc-angle-mode rad)
-                            ("F" calc-prefer-frac t)
-                            ("S" calc-symbolic-mode t)))
-           value (nth 2 var) var (nth 1 var)))
-  (if (memq var org-tbl-calc-modes)
-      (setcar (cdr (memq var org-tbl-calc-modes)) value)
-    (cons var (cons value org-tbl-calc-modes)))
-  org-tbl-calc-modes)
+(defsubst org-table--set-calc-mode (var value)
+  (plist-put org-tbl-calc-modes var value))
 
 
 ;;; Predicates
@@ -2476,9 +2467,14 @@ location of point."
                (setq keep-empty t
                      fmt (replace-match "" t t fmt)))
            (while (string-match "[DRFS]" fmt)
-             (setq org-tbl-calc-modes
-                   (org-table--set-calc-mode (match-string 0 fmt)))
-             (setq fmt (replace-match "" t t fmt)))
+             (let* ((c (string-to-char (match-string 0 fmt)))
+                    (mode (cdr (assoc c '((?D calc-angle-mode deg)
+                                          (?R calc-angle-mode rad)
+                                          (?F calc-prefer-frac t)
+                                          (?S calc-symbolic-mode t))))))
+               (setq org-tbl-calc-modes (org-table--set-calc-mode
+                                         (car mode) (cadr mode))
+                     fmt (replace-match "" t t fmt))))
            (unless (string-match "\\S-" fmt)
              (setq fmt nil))))
       (when (and (not suppress-const) org-table-formula-use-constants)
-- 
2.28.0

From fb8b62e5faabca2b6c6514e25bd306f7a5e8696f Mon Sep 17 00:00:00 2001
From: Daniele Nicolodi <dani...@grinta.net>
Date: Tue, 20 Oct 2020 15:13:40 +0200
Subject: [PATCH 2/4] org-table: Simplify mode string parsing

* org-table.el (org-table-eval-formula): Simplify mode string parsing
and reduce scope of local variables.
---
 lisp/org-table.el | 98 +++++++++++++++++++++--------------------------
 1 file changed, 43 insertions(+), 55 deletions(-)

diff --git a/lisp/org-table.el b/lisp/org-table.el
index 0790dc3ca..4baad2600 100644
--- a/lisp/org-table.el
+++ b/lisp/org-table.el
@@ -722,7 +722,7 @@ Field is restored even in case of abnormal exit."
         (set-marker ,line nil)))))
 
 (defsubst org-table--set-calc-mode (var value)
-  (plist-put org-tbl-calc-modes var value))
+  (setq org-tbl-calc-modes (plist-put org-tbl-calc-modes var value)))
 
 
 ;;; Predicates
@@ -2427,54 +2427,42 @@ location of point."
           (org-tbl-calc-modes (copy-sequence org-calc-default-modes))
           (numbers nil)           ; was a variable, now fixed default
           (keep-empty nil)
-          n form form0 formrpl formrg bw fmt x ev orig c lispp literal
+          form form0 formrpl formrg bw fmt ev orig lispp literal
           duration duration-output-format)
       ;; Parse the format string.  Since we have a lot of modes, this is
       ;; a lot of work.  However, I think calc still uses most of the time.
-      (if (string-match ";" formula)
-         (let ((tmp (org-split-string formula ";")))
-           (setq formula (car tmp)
-                 fmt (concat (cdr (assoc "%" org-table-local-parameters))
-                             (nth 1 tmp)))
+      (if (string-match "\\(.*\\);\\(.*\\)" formula)
+         (progn
+           (setq fmt (concat (match-string-no-properties 2 formula)
+                             (cdr (assoc "%" org-table-local-parameters)))
+                 formula (match-string-no-properties 1 formula))
            (while (string-match "\\([pnfse]\\)\\(-?[0-9]+\\)" fmt)
-             (setq c (string-to-char (match-string 1 fmt))
-                   n (string-to-number (match-string 2 fmt)))
-             (if (= c ?p)
-                 (setq org-tbl-calc-modes
-                       (org-table--set-calc-mode 'calc-internal-prec n))
-               (setq org-tbl-calc-modes
-                     (org-table--set-calc-mode
-                      'calc-float-format
-                      (list (cdr (assoc c '((?n . float) (?f . fix)
-                                            (?s . sci) (?e . eng))))
-                            n))))
+             (let ((c (string-to-char (match-string 1 fmt)))
+                   (n (string-to-number (match-string 2 fmt))))
+               (cl-case c
+                 (?p (org-table--set-calc-mode 'calc-internal-prec n))
+                 (?n (org-table--set-calc-mode 'calc-float-format (list 'float 
n)))
+                 (?f (org-table--set-calc-mode 'calc-float-format (list 'fix 
n)))
+                 (?s (org-table--set-calc-mode 'calc-float-format (list 'sci 
n)))
+                 (?e (org-table--set-calc-mode 'calc-float-format (list 'eng 
n)))))
+             ;; Remove matched flags from the mode string.
+             (setq fmt (replace-match "" t t fmt)))
+           (while (string-match "\\([tTUNLEDRFS]\\)" fmt)
+             (let ((c (string-to-char (match-string 1 fmt))))
+               (cl-case c
+                 (?t (setq duration t numbers t
+                           duration-output-format 
org-table-duration-custom-format))
+                 (?T (setq duration t numbers t duration-output-format nil))
+                 (?U (setq duration t numbers t duration-output-format 'hh:mm))
+                 (?N (setq numbers t))
+                 (?L (setq literal t))
+                 (?E (setq keep-empty t))
+                 (?D (org-table--set-calc-mode 'calc-angle-mode 'deg))
+                 (?R (org-table--set-calc-mode 'calc-angle-mode 'rad))
+                 (?F (org-table--set-calc-mode 'calc-prefer-frac t))
+                 (?S (org-table--set-calc-mode 'calc-symbolic-mode t))))
+             ;; Remove matched flags from the mode string.
              (setq fmt (replace-match "" t t fmt)))
-           (if (string-match "[tTU]" fmt)
-               (let ((ff (match-string 0 fmt)))
-                 (setq duration t numbers t
-                       duration-output-format
-                       (cond ((equal ff "T") nil)
-                             ((equal ff "t") org-table-duration-custom-format)
-                             ((equal ff "U") 'hh:mm))
-                       fmt (replace-match "" t t fmt))))
-           (if (string-match "N" fmt)
-               (setq numbers t
-                     fmt (replace-match "" t t fmt)))
-           (if (string-match "L" fmt)
-               (setq literal t
-                     fmt (replace-match "" t t fmt)))
-           (if (string-match "E" fmt)
-               (setq keep-empty t
-                     fmt (replace-match "" t t fmt)))
-           (while (string-match "[DRFS]" fmt)
-             (let* ((c (string-to-char (match-string 0 fmt)))
-                    (mode (cdr (assoc c '((?D calc-angle-mode deg)
-                                          (?R calc-angle-mode rad)
-                                          (?F calc-prefer-frac t)
-                                          (?S calc-symbolic-mode t))))))
-               (setq org-tbl-calc-modes (org-table--set-calc-mode
-                                         (car mode) (cadr mode))
-                     fmt (replace-match "" t t fmt))))
            (unless (string-match "\\S-" fmt)
              (setq fmt nil))))
       (when (and (not suppress-const) org-table-formula-use-constants)
@@ -2575,17 +2563,17 @@ location of point."
        (setq form0 form)
        ;; Insert the references to fields in same row
        (while (string-match "\\$\\(\\([-+]\\)?[0-9]+\\)" form)
-         (setq n (+ (string-to-number (match-string 1 form))
-                    (if (match-end 2) n0 0))
-               x (nth (1- (if (= n 0) n0 (max n 1))) fields)
-               formrpl (save-match-data
-                         (org-table-make-reference
-                          x keep-empty numbers lispp)))
-         (when (or (not x)
-                   (save-match-data
-                     (string-match (regexp-quote formula) formrpl)))
-           (user-error "Invalid field specifier \"%s\""
-                       (match-string 0 form)))
+         (let* ((n (+ (string-to-number (match-string 1 form))
+                      (if (match-end 2) n0 0)))
+                (x (nth (1- (if (= n 0) n0 (max n 1))) fields)))
+           (setq formrpl (save-match-data
+                           (org-table-make-reference
+                            x keep-empty numbers lispp)))
+           (when (or (not x)
+                     (save-match-data
+                       (string-match (regexp-quote formula) formrpl)))
+             (user-error "Invalid field specifier \"%s\""
+                         (match-string 0 form))))
          (setq form (replace-match formrpl t t form)))
 
        (if lispp
-- 
2.28.0

From aad8eb548e7a7a7fde1908a9f9c66f980da10b56 Mon Sep 17 00:00:00 2001
From: Daniele Nicolodi <dani...@grinta.net>
Date: Tue, 20 Oct 2020 15:22:02 +0200
Subject: [PATCH 3/4] org-table: Add mode flag to enable Calc units
 simplification mode

* org-table.el (org-table-eval-formula): Add the `u` mode flag to
enable Calc's units simplification mode.
---
 lisp/org-table.el | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lisp/org-table.el b/lisp/org-table.el
index 4baad2600..6b92656bd 100644
--- a/lisp/org-table.el
+++ b/lisp/org-table.el
@@ -2447,11 +2447,12 @@ location of point."
                  (?e (org-table--set-calc-mode 'calc-float-format (list 'eng 
n)))))
              ;; Remove matched flags from the mode string.
              (setq fmt (replace-match "" t t fmt)))
-           (while (string-match "\\([tTUNLEDRFS]\\)" fmt)
+           (while (string-match "\\([tuTUNLEDRFS]\\)" fmt)
              (let ((c (string-to-char (match-string 1 fmt))))
                (cl-case c
                  (?t (setq duration t numbers t
                            duration-output-format 
org-table-duration-custom-format))
+                 (?u (org-table--set-calc-mode 'calc-simplify-mode 'units))
                  (?T (setq duration t numbers t duration-output-format nil))
                  (?U (setq duration t numbers t duration-output-format 'hh:mm))
                  (?N (setq numbers t))
-- 
2.28.0

From 2d4521a032ec3e4174c97b2b2e9a08491e9870fb Mon Sep 17 00:00:00 2001
From: Daniele Nicolodi <dani...@grinta.net>
Date: Wed, 21 Oct 2020 17:47:15 +0200
Subject: [PATCH 4/4] org-table: Remove unused org-tbl-calc-modes variable

* org-table.el (org-tbl-calc-modes): Remove the variable declaration
as the varialble is only used as a local variable in `org-table-eval-formula'.

* org-table.el (org-table--set-calc-mode): Drop convenience macro.

* org-table.el (org-table-eval-formula): Rename `org-tbl-calc-modes`
local variable without the org-table prefix and usr the gained screen
real estate to avoid indirection through covenience macro.
---
 lisp/org-table.el | 29 ++++++++++++-----------------
 1 file changed, 12 insertions(+), 17 deletions(-)

diff --git a/lisp/org-table.el b/lisp/org-table.el
index 6b92656bd..1651decd3 100644
--- a/lisp/org-table.el
+++ b/lisp/org-table.el
@@ -676,8 +676,6 @@ Will be filled automatically during use.")
     ("_" . "Names for values in row below this one.")
     ("^" . "Names for values in row above this one.")))
 
-(defvar org-tbl-calc-modes nil)
-
 (defvar org-pos nil)
 
 
@@ -721,9 +719,6 @@ Field is restored even in case of abnormal exit."
         (org-table-goto-column ,column)
         (set-marker ,line nil)))))
 
-(defsubst org-table--set-calc-mode (var value)
-  (setq org-tbl-calc-modes (plist-put org-tbl-calc-modes var value)))
-
 
 ;;; Predicates
 
@@ -2424,7 +2419,7 @@ location of point."
                        equation
                      (org-table-get-formula equation (equal arg '(4)))))
           (n0 (org-table-current-column))
-          (org-tbl-calc-modes (copy-sequence org-calc-default-modes))
+          (calc-modes (copy-sequence org-calc-default-modes))
           (numbers nil)           ; was a variable, now fixed default
           (keep-empty nil)
           form form0 formrpl formrg bw fmt ev orig lispp literal
@@ -2440,11 +2435,11 @@ location of point."
              (let ((c (string-to-char (match-string 1 fmt)))
                    (n (string-to-number (match-string 2 fmt))))
                (cl-case c
-                 (?p (org-table--set-calc-mode 'calc-internal-prec n))
-                 (?n (org-table--set-calc-mode 'calc-float-format (list 'float 
n)))
-                 (?f (org-table--set-calc-mode 'calc-float-format (list 'fix 
n)))
-                 (?s (org-table--set-calc-mode 'calc-float-format (list 'sci 
n)))
-                 (?e (org-table--set-calc-mode 'calc-float-format (list 'eng 
n)))))
+                 (?p (setf (cl-getf calc-modes 'calc-internal-prec) n))
+                 (?n (setf (cl-getf calc-modes 'calc-float-format) (list 
'float n)))
+                 (?f (setf (cl-getf calc-modes 'calc-float-format) (list 'fix 
n)))
+                 (?s (setf (cl-getf calc-modes 'calc-float-format) (list 'sci 
n)))
+                 (?e (setf (cl-getf calc-modes 'calc-float-format) (list 'eng 
n)))))
              ;; Remove matched flags from the mode string.
              (setq fmt (replace-match "" t t fmt)))
            (while (string-match "\\([tuTUNLEDRFS]\\)" fmt)
@@ -2452,16 +2447,16 @@ location of point."
                (cl-case c
                  (?t (setq duration t numbers t
                            duration-output-format 
org-table-duration-custom-format))
-                 (?u (org-table--set-calc-mode 'calc-simplify-mode 'units))
+                 (?u (setf (cl-getf calc-modes 'calc-simplify-mode) 'units))
                  (?T (setq duration t numbers t duration-output-format nil))
                  (?U (setq duration t numbers t duration-output-format 'hh:mm))
                  (?N (setq numbers t))
                  (?L (setq literal t))
                  (?E (setq keep-empty t))
-                 (?D (org-table--set-calc-mode 'calc-angle-mode 'deg))
-                 (?R (org-table--set-calc-mode 'calc-angle-mode 'rad))
-                 (?F (org-table--set-calc-mode 'calc-prefer-frac t))
-                 (?S (org-table--set-calc-mode 'calc-symbolic-mode t))))
+                 (?D (setf (cl-getf calc-modes 'calc-angle-mode) 'deg))
+                 (?R (setf (cl-getf calc-modes 'calc-angle-mode) 'rad))
+                 (?F (setf (cl-getf calc-modes 'calc-prefer-frac) t))
+                 (?S (setf (cl-getf calc-modes 'calc-symbolic-mode) t))))
              ;; Remove matched flags from the mode string.
              (setq fmt (replace-match "" t t fmt)))
            (unless (string-match "\\S-" fmt)
@@ -2606,7 +2601,7 @@ location of point."
 
          (setq ev (if (and duration (string-match 
"^[0-9]+:[0-9]+\\(?::[0-9]+\\)?$" form))
                       form
-                    (calc-eval (cons form org-tbl-calc-modes)
+                    (calc-eval (cons form calc-modes)
                                (when (and (not keep-empty) numbers) 'num)))
                ev (if duration (org-table-time-seconds-to-string
                                 (if (string-match 
"^[0-9]+:[0-9]+\\(?::[0-9]+\\)?$" ev)
-- 
2.28.0

Reply via email to