bug#73005: [REGRESSION, BISECTED]: line numbers disappear when pressing `df` in evil-mode

[Konstantin Kharlamov]

On Mon, 2024-11-04 at 20:39 +0100, David Ponce wrote:
> I forgot to reply to all. Sorry.
>   
> On 04/11/2024 5:38 PM, Eli Zaretskii wrote:
> > > Date: Mon, 4 Nov 2024 14:58:09 +0100
> > > Cc: Hi-Angel@yandex.ru, 73005@debbugs.gnu.org
> > > From: David Ponce <da_vid@orange.fr>
> > > 
> > > > > 2024-11-04  David Ponce  <da_vid@orange.fr>
> > > > > 
> > > > >       * subr-x.el (string-pixel-width): Set text property
> > > > >       `display-line-numbers-disable' instead of changing
> > > > > the
> > > > >       buffer-local value of `display-line-numbers'.  Fix
> > > > > bug#73005.
> > > > 
> > > > Thanks, I will look into this when I have time.  It isn't
> > > > urgent.
> > > 
> > > No problem for me.  At the same time, maybe you can shed some
> > > light
> > > on why initializing the local variable `display-line-numbers' in
> > > the
> > > work buffer interferes with the setting in other buffer?  I'm
> > > probably
> > > missing something in the current implementation.
> > 
> > Without looking at the code, my first guess would be that you
> > change
> > the global default value, not the buffer-local value.  The fact
> > that
> > you kill-all-local-variables might have something to do with that.
> 
> I cannot see where it could happen in the code.
> 
> However, I think I have more or less figured out what could be going
> on.
> 
> `display-line-numbers' is a special variable that when changed causes
> the
> current buffer to be redisplayed in the current window (see frame.el
> from
> line 3194).
> 
> It seems that, when `string-pixels-width' sets the value of
> `display-line-numbers' to nil locally in the working buffer, this
> triggers
> a redisplay of the buffer in the selected window, and the line
> numbers to
> disappear.  When the working buffer is later recycled and local
> variables
> killed, it seems that this does not trigger buffer redisplay, and the
> line
> numbers therefore remain disabled, as observed in the simple recipe
> provided
> by Konstantin.

Thank you, great research! But note that problem is not triggered by
setting `display-line-numbers` to nil (even though it's part of the
equation). Instead it is triggered specifically by (kill-all-local-
variables), and removing this call makes problem go away.

>   If you just evaluate (setq display-line-numbers display-line-
> numbers)
> after Konstantin's recipe, the current buffer is redisplayed and the
> line
> numbers appear again, because `string-pixel-width' didn't change the
> global value
> of `display-line-numbers'.

A more curious way to get line numbers is evaluating just `display-
line-numbers`, i.e. without any modification.

> If I modify `string-pixel-width' to add
> `(setq display-line-numbers display-line-numbers)' before to return
> the result,
> line numbers are not deactivated:
> 
> (defun string-pixel-width (string &optional buffer)
>    "Return the width of STRING in pixels.
> If BUFFER is non-nil, use the face remappings from that buffer when
> determining the width.
> If you call this function to measure pixel width of a string
> with embedded newlines, it returns the width of the widest
> substring that does not include newlines."
>    (declare (important-return-value t))
>    (if (zerop (length string))
>        0
>      ;; Keeping a work buffer around is more efficient than creating
> a
>      ;; new temporary buffer.
>      (prog1
>          (with-work-buffer
>        ;; If `display-line-numbers' is enabled in internal
>        ;; buffers (e.g. globally), it breaks width calculation
>        ;; (bug#59311).  Disable `line-prefix' and `wrap-prefix',
>        ;; for the same reason.
>        (setq display-line-numbers nil
>              line-prefix nil wrap-prefix nil)
>        (if buffer
>            (setq-local face-remapping-alist
>                        (with-current-buffer buffer
>                          face-remapping-alist))
>          (kill-local-variable 'face-remapping-alist))
>        ;; Avoid deactivating the region as side effect.
>        (let (deactivate-mark)
>          (insert string))
>        ;; Prefer `remove-text-properties' to `propertize' to avoid
>        ;; creating a new string on each call.
>        (remove-text-properties
>         (point-min) (point-max) '(line-prefix nil wrap-prefix nil))
>        (car (buffer-text-pixel-size nil nil t)))
>        (setq display-line-numbers display-line-numbers))))
> 
> However, using the `display-line-numbers-disable' property to
> temporarily
> disable line numbers in `string-pixels-width', without involving
> redisplay,
> seems a better approach to me.
> 
> Regarding Konstantin recipe, `string-pixel-width' is called by `tab-
> bar-auto-width'
> when (lookup-key global-map [tab-bar]) is evaluated.
> 
> Thanks