unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#41513: `compute-motion' can miscount buffer positions in the presence of 'before-string/'after-string overlays
@ 2020-05-24 18:30 Stephen Bach
  2020-05-24 19:09 ` Eli Zaretskii
  0 siblings, 1 reply; 9+ messages in thread
From: Stephen Bach @ 2020-05-24 18:30 UTC (permalink / raw)
  To: 41513


[-- Attachment #1.1: Type: text/plain, Size: 2973 bytes --]

Hi,

Please see the attached demonstration code, runnable with `emacs -Q' and
`eval-buffer'.

When `compute-motion' scans along a buffer range that covers an overlay
with a 'buffer-string or 'after-string property [*] having as its content a
sort-of tricky but not too tricky propertized string (described below),
`compute-motion' can/will return an incorrect POS value.

E.g. an overlay with this plist:

  '(before-string #("AAA" 0 3 (display ""))

This overlay is effectively a no-op for redisplay - the "AAA" string that
would have been printed at the start of the overlay interval is not printed
because its presentation is suppressed by the string's 'display property.
This is right and proper.

However, `compute-motion' appears to interpret this overlay as having an
effective visible length of -3 rather than 0. Ditto if the overlay had
instead had an 'after-string property with the same content. [*] The
effective length being negative suggests to me an overcorrection rather
than missing logic.

([*] Aside: I believe `compute-motion' sometimes also miscalculates
'display as an overlay property for cases where one might reasonably expect
it to work- but I limit this report to 'before-string and 'after-string.)

Popular packages such as flycheck and git-gutter use this pattern of
overlay to annotate the margin and the fringe. The pattern is increasingly
common which in turn increasingly limits the application of
`compute-motion' as a reliable exposed function.

The miscounting appears to exist at least as far back as Emacs 24. Might
there be a mitigation/workaround? I like `compute-motion' despite its
complicated signature and complicated return value (and despite its
apparently rare use in the extended ecosystem) - it's fast and it involves
no cursor movement to perform its measurements.

Thanks for reading.


Testcase output:
=======================================

-------------------------------------
Without overlay:
- ‘point’ at col 30: <31>
- buffer content on and after col 30: <30   35   40   45   50>
- posn coord at col 30: <(30 . 0)>
- buffer pos of col 30 according to ‘compute-motion’: <31>     [ok]
- coord of col 30 according to ‘compute-motion’: <(30 . 0)>
-------------------------------------
With ’before-string overlay:
- ‘point’ at col 30: <31>
- buffer content on and after col 30: <30   35   40   45   50>
- posn coord at col 30: <(30 . 0)>
- buffer pos of col 30 according to ‘compute-motion’: <28>     <<<
INCONSISTENT
- coord of col 30 according to ‘compute-motion’: <(30 . 0)>
-------------------------------------
With ’after-string overlay:
- ‘point’ at col 30: <31>
- buffer content on and after col 30: <30   35   40   45   50>
- posn coord at col 30: <(30 . 0)>
- buffer pos of col 30 according to ‘compute-motion’: <28>     <<<
INCONSISTENT
- coord of col 30 according to ‘compute-motion’: <(30 . 0)>

[-- Attachment #1.2: Type: text/html, Size: 3333 bytes --]

[-- Attachment #2: compute-motion-testcase.el --]
[-- Type: application/octet-stream, Size: 3034 bytes --]

;;; -*- mode: lisp-interaction -*-

;; Summary: use `compute-motion' to measure the buffer position of the visual
;; coordinate '(0 . 30) in a new buffer with some content. Here, column 30 is
;; at (after) buffer position 31.
;;
;; `compute-motion' calculates this correctly unless it scans over a
;; 'before-string or 'after-string overlay of the style shown in
;; `testcase-apply-overlay' in which case it returns buffer position 28.

(defun testcase-recreate-buffer (testcase-buffer)
  (when (get-buffer testcase-buffer)
    (kill-buffer testcase-buffer))
  (with-current-buffer (get-buffer-create testcase-buffer)
    (insert "     5    10   15   20   25   30   35   40   45   50\n")
    (goto-char (point-min))))

(defun testcase-measure-at-col-30 (testcase-buffer)
  (let ((prev-buffer (current-buffer)))
    (switch-to-buffer testcase-buffer)
    (goto-char (point-min))
    (move-to-column 30)
    (redisplay)  ; hack: make posn-at-point return non-nil
    (message "- `point' at col 30: <%s>" (point))
    (message "- buffer content on and after col 30: <%s>"
             (buffer-substring (point) (line-end-position)))
    (message "- posn coord at col 30: <%s>" (posn-col-row (posn-at-point)))
    (pcase-let ((`(,pos ,col ,row _ _)
                 (compute-motion (point-min)
                                 '(0 . 0)
                                 (point-max)
                                 (posn-col-row (posn-at-point))
                                 nil nil nil)))
      (message "- buffer pos of col 30 according to `compute-motion': <%s> %s"
               pos
               (if (eq pos (point))
                   "    [ok]"
                 "    <<< INCONSISTENT"))
      (message "- coord of col 30 according to `compute-motion': <%s>"
               (cons col row)))
    (switch-to-buffer prev-buffer)))

(defun testcase-apply-overlay (buf overlay-pos prop-sym)
  (with-current-buffer buf
    (let* ((ol (make-overlay overlay-pos overlay-pos
                             (current-buffer) nil nil)))
      (overlay-put ol
                   prop-sym
                   (propertize "AAA" 'display ""))
      ol)))

(let ((testcase-buffer "*compute-motion bug*"))
  (message "\n=======================================\n")
  (let ((prev-buffer (current-buffer)))
    (message "-------------------------------------")
    (message "Without overlay:")
    (testcase-recreate-buffer testcase-buffer)
    (testcase-measure-at-col-30 testcase-buffer)

    (message "-------------------------------------")
    (message "With 'before-string overlay:")
    (testcase-recreate-buffer testcase-buffer)
    (testcase-apply-overlay testcase-buffer 20 'before-string)
    (testcase-measure-at-col-30 testcase-buffer)

    (message "-------------------------------------")
    (message "With 'after-string overlay:")
    (testcase-recreate-buffer testcase-buffer)
    (testcase-apply-overlay testcase-buffer 20 'after-string)
    (testcase-measure-at-col-30 testcase-buffer)

    (switch-to-buffer prev-buffer))
  nil)


^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#41513: `compute-motion' can miscount buffer positions in the presence of 'before-string/'after-string overlays
  2020-05-24 18:30 bug#41513: `compute-motion' can miscount buffer positions in the presence of 'before-string/'after-string overlays Stephen Bach
@ 2020-05-24 19:09 ` Eli Zaretskii
  2020-05-24 19:55   ` Stephen Bach
  0 siblings, 1 reply; 9+ messages in thread
From: Eli Zaretskii @ 2020-05-24 19:09 UTC (permalink / raw)
  To: Stephen Bach; +Cc: 41513

> From: Stephen Bach <sjbach@sjbach.com>
> Date: Sun, 24 May 2020 14:30:13 -0400
> 
> The miscounting appears to exist at least as far back as Emacs 24. Might there be a mitigation/workaround?
> I like `compute-motion' despite its complicated signature and complicated return value (and despite its
> apparently rare use in the extended ecosystem) - it's fast and it involves no cursor movement to perform its
> measurements.

Don't use compute-motion.  It is a remnant of the old pre-Emacs 21
display engine, and almost certainly doesn't support tricky display
features.

There are other functions that can do the same job.  If you describe
what you need to do, I could try suggesting some alternatives that
will work.





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#41513: `compute-motion' can miscount buffer positions in the presence of 'before-string/'after-string overlays
  2020-05-24 19:09 ` Eli Zaretskii
@ 2020-05-24 19:55   ` Stephen Bach
  2020-05-25 15:23     ` Eli Zaretskii
  0 siblings, 1 reply; 9+ messages in thread
From: Stephen Bach @ 2020-05-24 19:55 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 41513

On Sun, May 24, 2020 at 3:09 PM Eli Zaretskii <eliz@gnu.org> wrote:
> Don't use compute-motion.  It is a remnant of the old pre-Emacs 21
> display engine, and almost certainly doesn't support tricky display
> features.
>
> There are other functions that can do the same job.  If you describe
> what you need to do, I could try suggesting some alternatives that
> will work.

Thanks Eli. Specifically, I'm measuring the jagged contour of a buffer's
layout within a particular window, i.e. relating the final column of
each visual row (as displayed within the window) to concrete buffer
positions. This is notionally straightforward but complicated in
practice, as you will know -- e.g. line continuation/truncation,
`word-wrap', `visual-line-mode', `adaptive-wrap-prefix-mode',
multi-columnar glyphs. Text properties or overlays using 'display, and
so on.

In my initial go at this I used `end-of-visual-line' calls within a
`save-excursion' but I found that to be a little slow given the
frequency I was making the calls and I was also uncomfortable about all
the cursor movement, perhaps irrationally. But maybe this is the correct
approach?

Re: `compute-motion', perhaps the manual or the docstring could include
a short note to dissuade developers from using it? Its complicated
interface may be sufficient to keep most people away, but on the other
hand a complicated interface can suggest efficiency and intricate
functionality.





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#41513: `compute-motion' can miscount buffer positions in the presence of 'before-string/'after-string overlays
  2020-05-24 19:55   ` Stephen Bach
@ 2020-05-25 15:23     ` Eli Zaretskii
  2020-05-25 20:16       ` Stephen Bach
  0 siblings, 1 reply; 9+ messages in thread
From: Eli Zaretskii @ 2020-05-25 15:23 UTC (permalink / raw)
  To: Stephen Bach; +Cc: 41513

> From: Stephen Bach <sjbach@sjbach.com>
> Date: Sun, 24 May 2020 15:55:09 -0400
> Cc: 41513@debbugs.gnu.org
> 
> > There are other functions that can do the same job.  If you describe
> > what you need to do, I could try suggesting some alternatives that
> > will work.
> 
> Thanks Eli. Specifically, I'm measuring the jagged contour of a buffer's
> layout within a particular window, i.e. relating the final column of
> each visual row (as displayed within the window) to concrete buffer
> positions. This is notionally straightforward but complicated in
> practice, as you will know -- e.g. line continuation/truncation,
> `word-wrap', `visual-line-mode', `adaptive-wrap-prefix-mode',
> multi-columnar glyphs. Text properties or overlays using 'display, and
> so on.

Please tell more.  What exactly would you like to measure?  Is it the
width of each screen line? or just the maximum width of the longest
line among those shown in the window? or something else?





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#41513: `compute-motion' can miscount buffer positions in the presence of 'before-string/'after-string overlays
  2020-05-25 15:23     ` Eli Zaretskii
@ 2020-05-25 20:16       ` Stephen Bach
  2020-05-26 16:21         ` Eli Zaretskii
  0 siblings, 1 reply; 9+ messages in thread
From: Stephen Bach @ 2020-05-25 20:16 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 41513

On Mon, May 25, 2020 at 11:22 AM Eli Zaretskii <eliz@gnu.org> wrote:
>
> Please tell more.  What exactly would you like to measure?  Is it the
> width of each screen line? or just the maximum width of the longest
> line among those shown in the window? or something else?

The width of each screen line, kind-of. I'm writing a
terminal-compatible drawing library for displaying low-res
visualizations in the buffer/window:

   - https://sjbach.com/canvas-emacs-logo.mp4
   - https://sjbach.com/canvas-mario.mp4

To draw an image I apply temporary overlays to buffer ranges that
correspond to window coordinates which themselves correspond to the
"pixels" of the image.

A performance-sensitive part of this turns out to be when I walk between
`window-start' and `window-end' to figure out (1) the buffer positions
of the final characters of visible screen lines/visual rows, and (2) the
buffer positions underlying particular window coordinates that happen to
fall within the buffer text -- both of which I've been using
`compute-motion' to calculate.

Is `vertical-motion' a more reliable analogue to `compute-motion'? I see
it can take a (COLS . LINES) argument, so it appears similar.





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#41513: `compute-motion' can miscount buffer positions in the presence of 'before-string/'after-string overlays
  2020-05-25 20:16       ` Stephen Bach
@ 2020-05-26 16:21         ` Eli Zaretskii
  2020-05-26 18:12           ` Stephen Bach
  0 siblings, 1 reply; 9+ messages in thread
From: Eli Zaretskii @ 2020-05-26 16:21 UTC (permalink / raw)
  To: Stephen Bach; +Cc: 41513

> From: Stephen Bach <sjbach@sjbach.com>
> Date: Mon, 25 May 2020 16:16:08 -0400
> Cc: 41513@debbugs.gnu.org
> 
> Is `vertical-motion' a more reliable analogue to `compute-motion'? I see
> it can take a (COLS . LINES) argument, so it appears similar.

Depending on what exactly do you need to do, I can suggest the
following APIs:

  vertical-motion
  posn-at-point
  pos-visible-in-window-p
  window-text-pixel-size

Let me know if these don't cover the functionalities that you need.





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#41513: `compute-motion' can miscount buffer positions in the presence of 'before-string/'after-string overlays
  2020-05-26 16:21         ` Eli Zaretskii
@ 2020-05-26 18:12           ` Stephen Bach
  2020-05-26 19:09             ` Eli Zaretskii
  0 siblings, 1 reply; 9+ messages in thread
From: Stephen Bach @ 2020-05-26 18:12 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 41513

On Tue, May 26, 2020 at 12:21 PM Eli Zaretskii <eliz@gnu.org> wrote:
>
> Depending on what exactly do you need to do, I can suggest the
> following APIs:
>
>   vertical-motion
>   posn-at-point
>   pos-visible-in-window-p
>   window-text-pixel-size
>
> Let me know if these don't cover the functionalities that you need.

I overlooked pos-visible-in-window-p before, thanks for the reference.

Do you have any points of advice for avoiding side-effects when moving
the cursor around the buffer programmatically?

What I have so far:
- Wrap in save-excursion
- Bind to t:
  - inhibit-point-motion-hooks
  - cursor-sensor-inhibit
  - inhibit-field-text-motion (for precision, might be superfluous)
- Bind to nil:
  - goal-column (probably superfluous)

Thanks Eli, I appreciate the help and also the work you do for Emacs.





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#41513: `compute-motion' can miscount buffer positions in the presence of 'before-string/'after-string overlays
  2020-05-26 18:12           ` Stephen Bach
@ 2020-05-26 19:09             ` Eli Zaretskii
  2020-08-24  0:26               ` Stefan Kangas
  0 siblings, 1 reply; 9+ messages in thread
From: Eli Zaretskii @ 2020-05-26 19:09 UTC (permalink / raw)
  To: Stephen Bach; +Cc: 41513

> From: Stephen Bach <sjbach@sjbach.com>
> Date: Tue, 26 May 2020 14:12:10 -0400
> Cc: 41513@debbugs.gnu.org
> 
> Do you have any points of advice for avoiding side-effects when moving
> the cursor around the buffer programmatically?
> 
> What I have so far:
> - Wrap in save-excursion
> - Bind to t:
>   - inhibit-point-motion-hooks
>   - cursor-sensor-inhibit
>   - inhibit-field-text-motion (for precision, might be superfluous)
> - Bind to nil:
>   - goal-column (probably superfluous)

save-excursion should do, usually.

> Thanks Eli, I appreciate the help and also the work you do for Emacs.

You are welcome.





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#41513: `compute-motion' can miscount buffer positions in the presence of 'before-string/'after-string overlays
  2020-05-26 19:09             ` Eli Zaretskii
@ 2020-08-24  0:26               ` Stefan Kangas
  0 siblings, 0 replies; 9+ messages in thread
From: Stefan Kangas @ 2020-08-24  0:26 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Stephen Bach, 41513-done

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Stephen Bach <sjbach@sjbach.com>
>> Date: Tue, 26 May 2020 14:12:10 -0400
>> Cc: 41513@debbugs.gnu.org
>>
>> Do you have any points of advice for avoiding side-effects when moving
>> the cursor around the buffer programmatically?
>>
>> What I have so far:
>> - Wrap in save-excursion
>> - Bind to t:
>>   - inhibit-point-motion-hooks
>>   - cursor-sensor-inhibit
>>   - inhibit-field-text-motion (for precision, might be superfluous)
>> - Bind to nil:
>>   - goal-column (probably superfluous)
>
> save-excursion should do, usually.
>
>> Thanks Eli, I appreciate the help and also the work you do for Emacs.
>
> You are welcome.

From skimming this thread, it looks like all issues here were
resolved.  I'm therefore closing this bug report.

If this conclusion is incorrect, please reply to this email (use "Reply
to all" in your email client) and we can reopen the bug report.

Best regards,
Stefan Kangas





^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2020-08-24  0:26 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-05-24 18:30 bug#41513: `compute-motion' can miscount buffer positions in the presence of 'before-string/'after-string overlays Stephen Bach
2020-05-24 19:09 ` Eli Zaretskii
2020-05-24 19:55   ` Stephen Bach
2020-05-25 15:23     ` Eli Zaretskii
2020-05-25 20:16       ` Stephen Bach
2020-05-26 16:21         ` Eli Zaretskii
2020-05-26 18:12           ` Stephen Bach
2020-05-26 19:09             ` Eli Zaretskii
2020-08-24  0:26               ` Stefan Kangas

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.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).