emacs-orgmode
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH] Re: [BUG] org-colview tests fail in Emacs 29 because of wrong o


From: Ihor Radchenko
Subject: [PATCH] Re: [BUG] org-colview tests fail in Emacs 29 because of wrong org-colview's assumptions about current-column [9.5.4 (release_9.5.4-626-g45f9d8.dirty @ /home/yantar92/.emacs.d/straight/build/org/)]
Date: Sat, 30 Jul 2022 12:14:28 +0800

Ihor Radchenko <yantar92@gmail.com> writes:

> On Emacs 29, make test will fail on both bugfix and main:
>
> 5 unexpected results:
>    FAILED  test-org-colview/columns-move-left
>    FAILED  test-org-colview/columns-move-right
>    FAILED  test-org-colview/columns-new
>    FAILED  test-org-colview/columns-next-allowed-value
>    FAILED  test-org-colview/columns-update

I am attaching a tentative fix.

Also, note that `current-column' is used, for example, in org-element
list parser. If users use some kind of weird font-lock that replaces
indentation in Org lists, our parser may become faulty. Similarly,
`current-column' is being used in few more places across Org.

I am not yet fixing such possible issues, but it would be helpful if
someone comments on the considered possibilities.

Best,
Ihor

>From 8f56cf22ff1727542a237a2ed39d48eca1479414 Mon Sep 17 00:00:00 2001
Message-Id: 
<8f56cf22ff1727542a237a2ed39d48eca1479414.1659154264.git.yantar92@gmail.com>
From: Ihor Radchenko <yantar92@gmail.com>
Date: Sat, 30 Jul 2022 12:07:14 +0800
Subject: [PATCH] org-colview: Do not rely on `current-column' ignoring display
 properties

* lisp/org-macs.el (org-current-text-column): New macro calculating
current column without accounting display text properties.

* lisp/org-colview.el (org-columns-check-computed):
(org-columns-next-allowed-value):
(org-columns-new):
(org-columns-delete):
(org-columns-edit-attributes):
(org-columns-widen):
(org-columns-move-right):
(org-columns-move-left):
(org-columns-update): Use the new macro when calculating point
position in the column view table overlay.  Do _not_ use the new
macro when we want to get the visual column position of the point.

Fixes "test-org-colview/" failures on Emacs 29 after Emacs commit
4243747b1b8c3b7e3463822804b32e83febe2878:

;; Fix 'current-column' in the presence of display strings

;; * src/indent.c (check_display_width): Support calculation of width
;; of 'display' properties whose values are strings.  This fixes the
;; value returned by 'current-column' when display strings are
;; present between BOL and point.  (Bug#53795)

See 
https://orgmode.org/list/CACnOyijQc7BDDtrYQb+=VoGWkpWAyMu7O4qsvGpsU6SCgwiM8Q@mail.gmail.com
---
 lisp/org-colview.el | 25 +++++++++++++------------
 lisp/org-macs.el    |  5 +++++
 2 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/lisp/org-colview.el b/lisp/org-colview.el
index 13643101b..2829678f5 100644
--- a/lisp/org-colview.el
+++ b/lisp/org-colview.el
@@ -557,7 +557,7 @@ (defun org-columns-quit ()
 
 (defun org-columns-check-computed ()
   "Throw an error if current column value is computed."
-  (let ((spec (nth (current-column) org-columns-current-fmt-compiled)))
+  (let ((spec (nth (org-current-text-column) 
org-columns-current-fmt-compiled)))
     (and
      (nth 3 spec)
      (assoc spec (get-text-property (line-beginning-position) 'org-summaries))
@@ -713,7 +713,8 @@ (defun org-columns-next-allowed-value (&optional previous 
nth)
 an integer, select that value."
   (interactive)
   (org-columns-check-computed)
-  (let* ((column (current-column))
+  (let* ((column (org-current-text-column))
+         (visible-column (current-column))
         (key (get-char-property (point) 'org-columns-key))
         (value (get-char-property (point) 'org-columns-value))
         (pom (or (get-text-property (line-beginning-position) 'org-hd-marker)
@@ -763,7 +764,7 @@ (defun org-columns-next-allowed-value (&optional previous 
nth)
        ;; the right place on the current line.
        (let ((org-columns-inhibit-recalculation)) (org-columns-redo))
        (org-columns-update key)
-       (org-move-to-column column))))))
+       (org-move-to-column visible-column))))))
 
 (defun org-colview-construct-allowed-dates (s)
   "Construct a list of three dates around the date in S.
@@ -925,14 +926,14 @@ (defun org-columns-new (&optional spec &rest attributes)
     (if spec
        (progn (setcar spec (car new))
               (setcdr spec (cdr new)))
-      (push new (nthcdr (current-column) org-columns-current-fmt-compiled)))
+      (push new (nthcdr (org-current-text-column) 
org-columns-current-fmt-compiled)))
     (org-columns-store-format)
     (org-columns-redo)))
 
 (defun org-columns-delete ()
   "Delete the column at point from columns view."
   (interactive)
-  (let ((spec (nth (current-column) org-columns-current-fmt-compiled)))
+  (let ((spec (nth (org-current-text-column) 
org-columns-current-fmt-compiled)))
     (when (y-or-n-p (format "Are you sure you want to remove column %S? "
                            (nth 1 spec)))
       (setq org-columns-current-fmt-compiled
@@ -942,18 +943,18 @@ (defun org-columns-delete ()
       ;; updating it may prove counter-intuitive.  See comments in
       ;; `org-columns-move-right' for details.
       (let ((org-columns-inhibit-recalculation t)) (org-columns-redo))
-      (when (>= (current-column) (length org-columns-current-fmt-compiled))
+      (when (>= (org-current-text-column) (length 
org-columns-current-fmt-compiled))
        (backward-char)))))
 
 (defun org-columns-edit-attributes ()
   "Edit the attributes of the current column."
   (interactive)
-  (org-columns-new (nth (current-column) org-columns-current-fmt-compiled)))
+  (org-columns-new (nth (org-current-text-column) 
org-columns-current-fmt-compiled)))
 
 (defun org-columns-widen (arg)
   "Make the column wider by ARG characters."
   (interactive "p")
-  (let* ((n (current-column))
+  (let* ((n (org-current-text-column))
         (entry (nth n org-columns-current-fmt-compiled))
         (width (aref org-columns-current-maxwidths n)))
     (setq width (max 1 (+ width arg)))
@@ -969,7 +970,7 @@ (defun org-columns-narrow (arg)
 (defun org-columns-move-right ()
   "Swap this column with the one to the right."
   (interactive)
-  (let* ((n (current-column))
+  (let* ((n (org-current-text-column))
         (cell (nthcdr n org-columns-current-fmt-compiled))
         e)
     (when (>= n (1- (length org-columns-current-fmt-compiled)))
@@ -993,7 +994,7 @@ (defun org-columns-move-right ()
 (defun org-columns-move-left ()
   "Swap this column with the one to the left."
   (interactive)
-  (let* ((n (current-column)))
+  (let* ((n (org-current-text-column)))
     (when (= n 0)
       (error "Cannot shift this column further to the left"))
     (backward-char 1)
@@ -1039,7 +1040,7 @@ (defun org-columns-update (property)
        (let ((key (overlay-get ov 'org-columns-key)))
         (when (and key (equal key p) (overlay-start ov))
           (goto-char (overlay-start ov))
-          (let* ((spec (nth (current-column) org-columns-current-fmt-compiled))
+          (let* ((spec (nth (org-current-text-column) 
org-columns-current-fmt-compiled))
                  (value
                   (or (cdr (assoc spec
                                   (get-text-property (line-beginning-position)
@@ -1049,7 +1050,7 @@ (defun org-columns-update (property)
               (let ((displayed (org-columns--displayed-value spec value))
                     (format (overlay-get ov 'org-columns-format))
                     (width
-                     (aref org-columns-current-maxwidths (current-column))))
+                     (aref org-columns-current-maxwidths 
(org-current-text-column))))
                 (overlay-put ov 'org-columns-value value)
                 (overlay-put ov 'org-columns-value-modified displayed)
                 (overlay-put ov
diff --git a/lisp/org-macs.el b/lisp/org-macs.el
index 1dc0b7080..44060f831 100644
--- a/lisp/org-macs.el
+++ b/lisp/org-macs.el
@@ -1037,6 +1037,11 @@ (defun org-string-width (string &optional pixels)
               pixel-width
             (/ pixel-width symbol-width)))))))
 
+(defmacro org-current-text-column ()
+  "Like `current-column' but ignore display properties."
+  `(string-width (buffer-substring-no-properties
+                  (line-beginning-position) (point))))
+
 (defun org-not-nil (v)
   "If V not nil, and also not the string \"nil\", then return V.
 Otherwise return nil."
-- 
2.35.1


reply via email to

[Prev in Thread] Current Thread [Next in Thread]