From: Daniele Nicolodi <daniele@grinta.net> To: emacs-orgmode@gnu.org Subject: [PATCH] org-table: Add mode flag to enable Calc units simplification mode Date: Tue, 20 Oct 2020 15:30:00 +0200 [thread overview] Message-ID: <33143b05-a298-8277-313b-3a1deb69ad27@grinta.net> (raw) In-Reply-To: <6d8c15c2-d1b0-d913-df39-c60381cff70b@grinta.net> [-- Attachment #1: Type: text/plain, Size: 1890 bytes --] Hello, attached there are a few patches reworking the code, fixing the bug, 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. Cheers, Dan On 19/10/2020 17:38, Daniele Nicolodi wrote: > Hello, > > I am hacking org-table-eval-formula (see thread about monetary values in > org-tables) which uses this inline function: > > (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) > > which I am not able to understand or which is not correct. > > The first (if ...) does some value substitutions, however, IIUC the > second (if ...) sets a new value for an entry in the org-tbl-calc-modes > plist if the entry is already present and builds a new plist with the > entry prepended if the entry is not there. However, the original plist > is returned and not the one with the new entry prepended. > > It does not seem to be the intended behavior. > > Shouldn't this be simply: > > (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))) > (plist-put org-tbl-calc-modes var value)) > > or, better, the code refactored to do not use this function? > > Cheers, > Dan > [-- Attachment #2: 0001-org-table-Fix-table-formula-mode-string-handling.patch --] [-- Type: text/plain, Size: 2160 bytes --] From c7434974897d932fe3acd182f06a98a61719e208 Mon Sep 17 00:00:00 2001 From: Daniele Nicolodi <daniele@grinta.net> Date: Tue, 20 Oct 2020 11:03:14 +0200 Subject: [PATCH 1/3] 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)) \f ;;; 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 [-- Attachment #3: 0003-org-table-Add-mode-flag-to-enable-Calc-units-simplif.patch --] [-- Type: text/plain, Size: 1301 bytes --] From aad8eb548e7a7a7fde1908a9f9c66f980da10b56 Mon Sep 17 00:00:00 2001 From: Daniele Nicolodi <daniele@grinta.net> Date: Tue, 20 Oct 2020 15:22:02 +0200 Subject: [PATCH 3/3] 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 [-- Attachment #4: 0002-org-table-Simplify-mode-string-parsing.patch --] [-- Type: text/plain, Size: 5850 bytes --] From fb8b62e5faabca2b6c6514e25bd306f7a5e8696f Mon Sep 17 00:00:00 2001 From: Daniele Nicolodi <daniele@grinta.net> Date: Tue, 20 Oct 2020 15:13:40 +0200 Subject: [PATCH 2/3] 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))) \f ;;; 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
next prev parent reply other threads:[~2020-10-20 13:31 UTC|newest] Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-10-19 15:38 Bug in org-table--set-calc-mode? Daniele Nicolodi 2020-10-20 13:30 ` Daniele Nicolodi [this message] 2020-10-20 13:44 ` [PATCH] org-table: Add mode flag to enable Calc units simplification mode Eric S Fraga 2020-10-20 14:00 ` Daniele Nicolodi 2020-10-20 14:22 ` Eric S Fraga 2020-10-20 14:19 ` Eric S Fraga 2020-10-20 14:32 ` Daniele Nicolodi 2020-10-20 14:53 ` Daniele Nicolodi 2020-10-20 15:35 ` Eric S Fraga 2020-10-20 15:35 ` Eric S Fraga 2020-10-21 15:57 ` Daniele Nicolodi 2020-10-24 15:33 Daniele Nicolodi 2020-11-07 14:03 ` Daniele Nicolodi 2020-11-19 5:58 ` Kyle Meyer 2020-11-19 20:02 ` Daniele Nicolodi 2020-11-23 3:14 ` Kyle Meyer 2020-11-23 10:22 ` Daniele Nicolodi 2020-11-23 22:27 ` Kyle Meyer 2020-11-24 0:03 ` Daniele Nicolodi 2020-11-24 5:35 ` Kyle Meyer 2020-11-24 8:05 ` Daniele Nicolodi 2020-11-25 2:07 ` Kyle Meyer 2020-11-25 7:41 ` Christian Moe 2020-11-23 10:25 ` Daniele Nicolodi 2020-11-23 22:25 ` Kyle Meyer 2020-11-23 23:01 ` Daniele Nicolodi
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=33143b05-a298-8277-313b-3a1deb69ad27@grinta.net \ --to=daniele@grinta.net \ --cc=emacs-orgmode@gnu.org \ --subject='Re: [PATCH] org-table: Add mode flag to enable Calc units simplification mode' \ /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
Code repositories for project(s) associated with this 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).