emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
From: Daniele Nicolodi <daniele@grinta.net>
To: emacs-orgmode@gnu.org
Subject: Re: [PATCH] org-table: Add mode flag to enable Calc units simplification mode
Date: Wed, 21 Oct 2020 17:57:01 +0200	[thread overview]
Message-ID: <5379f64a-e82c-ca75-0fe4-de14aea010ac@grinta.net> (raw)
In-Reply-To: <33143b05-a298-8277-313b-3a1deb69ad27@grinta.net>

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

Hello,

after working on this I realized that the org-tbl-calc-modes variables
is used only locally despite being declare globally. Maybe a remnant
from pre-lexical-binding times. Attached is a patch (on top of the
others one in this thread) that simplifies things a little.

Cheers,
Dan


On 20/10/2020 15:30, Daniele Nicolodi wrote:
> 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: 0004-org-table-Remove-unused-org-tbl-calc-modes-variable.patch --]
[-- Type: text/plain, Size: 4374 bytes --]

From 2d4521a032ec3e4174c97b2b2e9a08491e9870fb Mon Sep 17 00:00:00 2001
From: Daniele Nicolodi <daniele@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)
 
 \f
@@ -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)))
-
 \f
 ;;; 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


  parent reply	other threads:[~2020-10-21 15:57 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 ` [PATCH] org-table: Add mode flag to enable Calc units simplification mode Daniele Nicolodi
2020-10-20 13:44   ` 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 [this message]
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=5379f64a-e82c-ca75-0fe4-de14aea010ac@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).