all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#67248: 30.0.50; string-pixel-width is affected by the global value of display-line-numbers
@ 2023-11-17 21:23 Dmitry Gutov
  2023-11-18  7:14 ` Eli Zaretskii
  0 siblings, 1 reply; 3+ messages in thread
From: Dmitry Gutov @ 2023-11-17 21:23 UTC (permalink / raw)
  To: 67248

1. (string-pixel-width "abc") ; => 51
2. (customize-set-variable 'display-line-numbers t)
3. (string-pixel-width "abc") ; => 119

This function's implementation makes provision for
display-line-numbers-mode, but not for the original variable
display-line-numbers, which will be t independent of the value of
display-line-numbers-mode.

I think it's a bad idea to have these two different ways to do the same
thing, and it can lead to bugs like this one. I would suggest obsoleting
the public use of display-line-numbers -- making it private, probably.
At least removing it from the customization group display-line-numbers.

Barring that, the patch below seems to work.  Although it proliferates
the differences between Emacs versions which one would have to be on the
lookout for.

Originally reported at
https://github.com/company-mode/company-mode/issues/1427.

diff --git a/lisp/emacs-lisp/subr-x.el b/lisp/emacs-lisp/subr-x.el
index 572822351b1..4fda98ae3d0 100644
--- a/lisp/emacs-lisp/subr-x.el
+++ b/lisp/emacs-lisp/subr-x.el
@@ -343,8 +343,7 @@ string-pixel-width
      (with-current-buffer (get-buffer-create " *string-pixel-width*")
        ;; If `display-line-numbers-mode' is enabled in internal
        ;; buffers, it breaks width calculation, so disable it (bug#59311)
-      (when (bound-and-true-p display-line-numbers-mode)
-        (display-line-numbers-mode -1))
+      (setq-local display-line-numbers nil)
        (delete-region (point-min) (point-max))
        ;; Disable line-prefix and wrap-prefix, for the same reason.
        (setq line-prefix nil


In GNU Emacs 30.0.50 (build 271, x86_64-pc-linux-gnu, GTK+ Version
  3.24.34, cairo version 1.16.0) of 2023-11-17 built on potemkin
Repository revision: 94849ba35f40393362050b5937578dcf3076b4bf
Repository branch: feature/cl-lib-improvements
Windowing system distributor 'The X.Org Foundation', version 11.0.12101004
System Description: Ubuntu 22.10





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

* bug#67248: 30.0.50; string-pixel-width is affected by the global value of display-line-numbers
  2023-11-17 21:23 bug#67248: 30.0.50; string-pixel-width is affected by the global value of display-line-numbers Dmitry Gutov
@ 2023-11-18  7:14 ` Eli Zaretskii
  2023-11-18 16:36   ` Dmitry Gutov
  0 siblings, 1 reply; 3+ messages in thread
From: Eli Zaretskii @ 2023-11-18  7:14 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 67248

> Date: Fri, 17 Nov 2023 23:23:58 +0200
> From: Dmitry Gutov <dgutov@yandex.ru>
> 
> 1. (string-pixel-width "abc") ; => 51
> 2. (customize-set-variable 'display-line-numbers t)
> 3. (string-pixel-width "abc") ; => 119
> 
> This function's implementation makes provision for
> display-line-numbers-mode, but not for the original variable
> display-line-numbers, which will be t independent of the value of
> display-line-numbers-mode.
> 
> I think it's a bad idea to have these two different ways to do the same
> thing, and it can lead to bugs like this one. I would suggest obsoleting
> the public use of display-line-numbers -- making it private, probably.
> At least removing it from the customization group display-line-numbers.

I don't see a need for such drastic measures, certainly not due to
this bug.  Having the variable and the mode in this case is not
different from other similar cases, for example, word-wrap vs
visual-line-mode.  You have just discovered that the test in
string-pixel-width was not the correct one, that's all.

> Barring that, the patch below seems to work.  Although it proliferates
> the differences between Emacs versions which one would have to be on the
> lookout for.

Thanks, I think this is indeed the right solution.  Please install the
patch on the emacs-29 branch.





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

* bug#67248: 30.0.50; string-pixel-width is affected by the global value of display-line-numbers
  2023-11-18  7:14 ` Eli Zaretskii
@ 2023-11-18 16:36   ` Dmitry Gutov
  0 siblings, 0 replies; 3+ messages in thread
From: Dmitry Gutov @ 2023-11-18 16:36 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 67248-done

Version: 29.2

On 18/11/2023 09:14, Eli Zaretskii wrote:
> I don't see a need for such drastic measures, certainly not due to
> this bug.  Having the variable and the mode in this case is not
> different from other similar cases, for example, word-wrap vs
> visual-line-mode.

At least word-wrap and visual-line-mode have different names. And the 
latter provides a bunch of other features than the former.

display-line-numbers and display-line-numbers-mode sound totally 
identical, so I'm not at all surprised that the author of the referenced 
bug report customized both of them. That's a usability problem.

 > Thanks, I think this is indeed the right solution.  Please install the
patch on the emacs-29 branch.

Okay, pushed as 0128495afde. Thanks.





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

end of thread, other threads:[~2023-11-18 16:36 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-17 21:23 bug#67248: 30.0.50; string-pixel-width is affected by the global value of display-line-numbers Dmitry Gutov
2023-11-18  7:14 ` Eli Zaretskii
2023-11-18 16:36   ` Dmitry Gutov

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.