* [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/)]
@ 2022-07-15 0:44 Ihor Radchenko
2022-07-30 4:14 ` [PATCH] " Ihor Radchenko
0 siblings, 1 reply; 3+ messages in thread
From: Ihor Radchenko @ 2022-07-15 0:44 UTC (permalink / raw)
To: emacs-orgmode
[-- Attachment #1: Type: text/plain, Size: 1686 bytes --]
Hi,
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
This happens since 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)
There is nothing wrong in this Emacs commit.
However, org-colview.el, and particularly
(nth (current-column) org-columns-current-fmt-compiled)
statements, e.g. in org-columns-update (also in other places)
rely on current-column ignoring overlays and display properties.
To see the issue interactively, one can use the following example
recipe:
1. Run Emacs 29 master loading the latest bugfix/main Org branch
2. Open the following file:
* H
:PROPERTIES:
:A: 1
:END:
** S
:PROPERTIES:
:A: 2
:END:
3. Put point at the beginning of the first heading
4. Run M-: (let ((org-columns-default-format "%A %A{min}")) (org-columns))
5. Run M-: (org-columns-update "A")
6. Observe (error "Invalid column specification format: nil") caused by
org-columns-update trying to retrieve the column format using the
sexp relying of (current-column).
Note that fixing this bug will probably require checking the logic of
org-colview - one may need to dive deeply into that code.
Best,
Ihor
[-- Attachment #2: Type: text/html, Size: 1956 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
* [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/)]
2022-07-15 0:44 [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/)] Ihor Radchenko
@ 2022-07-30 4:14 ` Ihor Radchenko
2022-08-06 5:43 ` Ihor Radchenko
0 siblings, 1 reply; 3+ messages in thread
From: Ihor Radchenko @ 2022-07-30 4:14 UTC (permalink / raw)
To: emacs-orgmode
[-- Attachment #1: Type: text/plain, Size: 807 bytes --]
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
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org-colview-Do-not-rely-on-current-column-ignoring-d.patch --]
[-- Type: text/x-patch, Size: 6961 bytes --]
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [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/)]
2022-07-30 4:14 ` [PATCH] " Ihor Radchenko
@ 2022-08-06 5:43 ` Ihor Radchenko
0 siblings, 0 replies; 3+ messages in thread
From: Ihor Radchenko @ 2022-08-06 5:43 UTC (permalink / raw)
To: emacs-orgmode
Ihor Radchenko <yantar92@gmail.com> writes:
> 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.
Applied onto main via 5a1b05031.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=5a1b050310b484426007d050c0c30bedca49ee5b
--
Ihor Radchenko,
Org mode contributor,
Learn more about Org mode at https://orgmode.org/.
Support Org development at https://liberapay.com/org-mode,
or support my work at https://liberapay.com/yantar92
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-08-06 5:44 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-07-15 0:44 [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/)] Ihor Radchenko
2022-07-30 4:14 ` [PATCH] " Ihor Radchenko
2022-08-06 5:43 ` Ihor Radchenko
Code repositories for project(s) associated with this public 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).