Browse Source

org-colview: Do not compute values swapping columns

* lisp/org-colview.el (org-columns-move-right): Do not compute values
  swapping columns.  Comment about a corner case.

* testing/lisp/test-org-colview.el (test-org-colview/columns-move-left):
(test-org-colview/columns-move-right): New tests.
Nicolas Goaziou 5 years ago
parent
commit
acef7d8a43
2 changed files with 123 additions and 1 deletions
  1. 10 1
      lisp/org-colview.el
  2. 113 0
      testing/lisp/test-org-colview.el

+ 10 - 1
lisp/org-colview.el

@@ -896,7 +896,16 @@ When COLUMNS-FMT-STRING is non-nil, use it as the column format."
     (setcar cell (car (cdr cell)))
     (setcdr cell (cons e (cdr (cdr cell))))
     (org-columns-store-format)
-    (org-columns-redo)
+    ;; Do not compute again properties, since we're just moving
+    ;; columns around.  It can put a property value a bit off when
+    ;; switching between an non-computed and a computed value for the
+    ;; same property, e.g. from "%A %A{+}" to "%A{+} %A".
+    ;;
+    ;; In this case, the value needs to be updated since the first
+    ;; column related to a property determines how its value is
+    ;; computed.  However, (correctly) updating the value could be
+    ;; surprising, so we leave it as-is nonetheless.
+    (let ((org-columns-inhibit-recalculation t)) (org-columns-redo))
     (forward-char 1)))
 
 (defun org-columns-move-left ()

+ 113 - 0
testing/lisp/test-org-colview.el

@@ -846,6 +846,119 @@
 	    (list (get-char-property 1 'org-columns-value-modified)
 		  (get-char-property 2 'org-columns-value-modified))))))
 
+(ert-deftest test-org-colview/columns-move-left ()
+  "Test `org-columns-move-left' specifications."
+  ;; Error when trying to move the left-most column.
+  (should-error
+   (org-test-with-temp-text "* H"
+     (let ((org-columns-default-format "%ITEM")) (org-columns))
+     (org-columns-move-left)))
+  ;; Otherwise, move column to left and update display.
+  (should
+   (equal '("2" "1")
+	  (org-test-with-temp-text "* H\n:PROPERTIES:\n:A: 1\n:B: 2\n:END:"
+	    (let ((org-columns-default-format "%A %B")) (org-columns))
+	    (forward-char)
+	    (org-columns-move-left)
+	    (list (get-char-property (point) 'org-columns-value)
+		  (get-char-property (1+ (point)) 'org-columns-value)))))
+  ;; Handle multiple columns with the same property.
+  (should
+   (equal '("2" "1")
+	  (org-test-with-temp-text
+	      "* H
+** S1
+:PROPERTIES:
+:A: 1
+:END:
+** S1
+:PROPERTIES:
+:A: 2
+:END:"
+	    (let ((org-columns-default-format "%ITEM %A{min} %A{max}"))
+	      (org-columns))
+	    (forward-char 2)
+	    (org-columns-move-left)
+	    (list (get-char-property (point) 'org-columns-value)
+		  (get-char-property (1+ (point)) 'org-columns-value)))))
+  ;; Special case: do not update values even if move entails changing
+  ;; them.
+  (should
+   (equal "1"
+	  (org-test-with-temp-text
+	      "* H
+:PROPERTIES:
+:A: 1
+:END:
+** S1
+:PROPERTIES:
+:A: 99
+:END:"
+	    (let ((org-columns-default-format "%A %A{max}"))
+	      (org-columns))
+	    (forward-char)
+	    (org-columns-move-left)
+	    ;; Since the first column matching a given property
+	    ;; determines how a value is computed, the following
+	    ;; should return "99" instead.  However, this behavior is
+	    ;; in practice surprising so we just ignore the value
+	    ;; change.  It can be computed later.
+	    (org-entry-get (point) "A")))))
+
+(ert-deftest test-org-colview/columns-move-right ()
+  "Test `org-columns-move-right' specifications."
+  ;; Error when trying to move the right-most column.
+  (should-error
+   (org-test-with-temp-text "* H"
+     (let ((org-columns-default-format "%ITEM")) (org-columns))
+     (org-columns-move-right)))
+  ;; Otherwise, move column to left and update display.
+  (should
+   (equal '("2" "1")
+	  (org-test-with-temp-text "* H\n:PROPERTIES:\n:A: 1\n:B: 2\n:END:"
+	    (let ((org-columns-default-format "%A %B")) (org-columns))
+	    (org-columns-move-right)
+	    (list (get-char-property (1- (point)) 'org-columns-value)
+		  (get-char-property (point) 'org-columns-value)))))
+  ;; Handle multiple columns with the same property.
+  (should
+   (equal '("2" "1")
+	  (org-test-with-temp-text
+	      "* H
+** S1
+:PROPERTIES:
+:A: 1
+:END:
+** S1
+:PROPERTIES:
+:A: 2
+:END:"
+	    (let ((org-columns-default-format "%ITEM %A{min} %A{max}"))
+	      (org-columns))
+	    (forward-char)
+	    (org-columns-move-right)
+	    (list (get-char-property (1- (point)) 'org-columns-value)
+		  (get-char-property (point) 'org-columns-value)))))
+  ;; Special case: do not update values even if move entails changing
+  ;; them.
+  (should
+   (equal "1"
+	  (org-test-with-temp-text
+	      "* H
+:PROPERTIES:
+:A: 1
+:END:
+** S1
+:PROPERTIES:
+:A: 99
+:END:"
+	    (let ((org-columns-default-format "%A %A{max}"))
+	      (org-columns))
+	    (org-columns-move-right)
+	    ;; See `test-org-colview/columns-move-left' for an
+	    ;; explanation.
+	    (org-entry-get (point) "A")))))
+
 
 
 ;;; Dynamic block