bug#29597: 26.0.90; line-number-display-width returns incorrect width

bug#29597: 26.0.90; line-number-display-width returns incorrect width

[Paul Rankin]

[-- Attachment #1: Type: text/plain, Size: 627 bytes --]

After reading #28248[1] I’m not sure if this is now fixed on the release branch, but on 26.0.90 function line-number-display-width returns an off-by-2 incorrect value.

See attached screenshot or [2] also.

To reproduce:

1. emacs -Q (or emacs -Q -nw)
2. (display-line-numbers-mode)
3. (line-number-display-width)

Expected results:
4

Actual results:
2

Configuration:

GNU Emacs 26.0.90 (build 1, x86_64-apple-darwin17.2.0, NS appkit-1561.10 Version 10.13.1 (Build 17B48)) of 2017-11-05
macOS 10.13.1 (17B1003)

[1]: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=28248
[2]: https://imgur.com/a/kuZjA

[-- Attachment #2: Screen Shot 2017-12-07 at 3.10.16 pm.png --]
[-- Type: image/png, Size: 53857 bytes --]

bug#29597: 26.0.90; line-number-display-width returns incorrect width

[Eli Zaretskii]

On December 7, 2017 5:34:51 AM GMT+00:00, Paul Rankin <hello@paulwrankin.com> wrote:
> After reading #28248[1] I’m not sure if this is now fixed on the
> release branch, but on 26.0.90 function line-number-display-width
> returns an off-by-2 incorrect value.
> 
> See attached screenshot or [2] also.
> 
> To reproduce:
> 
> 1. emacs -Q (or emacs -Q -nw)
> 2. (display-line-numbers-mode)
> 3. (line-number-display-width)
> 
> Expected results:
> 4
> 
> Actual results:
> 2
> 
> Configuration:
> 
> GNU Emacs 26.0.90 (build 1, x86_64-apple-darwin17.2.0, NS
> appkit-1561.10 Version 10.13.1 (Build 17B48)) of 2017-11-05
> macOS 10.13.1 (17B1003)
> 
> [1]: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=28248
> [2]: https://imgur.com/a/kuZjA

This is not a bug, see the doc string on the release branch.  The latest code on the branch has more flexible API when the optional argument is non-nil.

bug#29597: 26.0.90; line-number-display-width returns incorrect width

[Paul Rankin]

I had a look in the source of display-line-numbers.el in the repo but cannot find the definition of line-number-display-width.

But this is irrelevant, the line number width is clearly 4 characters, lining up with File in the menu bar.

(string-width "File")
4

It clearly fails to count to four. Adding a docstring that 2 really equals 4 is absurd.

Please don’t close bugs until they’re actually closed.

bug#29597: 26.0.90; line-number-display-width returns incorrect width

[Noam Postavsky]

Paul Rankin <hello@paulwrankin.com> writes:

> I had a look in the source of display-line-numbers.el in the repo but cannot find the definition of line-number-display-width.

Here's the help text:

    line-number-display-width is a built-in function in ‘C source code’.

    (line-number-display-width &optional PIXELWISE)

    Return the width used for displaying line numbers in the selected window.
    If optional argument PIXELWISE is the symbol ‘columns’, return the width
    in units of the frame’s canonical character width.  In this case, the
    value is a float.
    If optional argument PIXELWISE is t or any other non-nil value, return
    the width as an integer number of pixels.
    Otherwise return the value as an integer number of columns of the face
    used to display line numbers, ‘line-number’.  Note that in the latter
    case, the value doesn’t include the 2 columns used for padding the
    numbers on display.

bug#29597: 26.0.90; line-number-display-width returns incorrect width

[Paul Rankin]

On Fri, 8 Dec 2017, at 12:00 PM, Noam Postavsky wrote:

> Here's the help text:
> 
>     line-number-display-width is a built-in function in ‘C source code’.
> 
>     (line-number-display-width &optional PIXELWISE)
> 
>     Return the width used for displaying line numbers in the selected window.
>     If optional argument PIXELWISE is the symbol ‘columns’, return the width
>     in units of the frame’s canonical character width.  In this case, the
>     value is a float.
>     If optional argument PIXELWISE is t or any other non-nil value, return
>     the width as an integer number of pixels.
>     Otherwise return the value as an integer number of columns of the face
>     used to display line numbers, ‘line-number’.  Note that in the latter
>     case, the value doesn’t include the 2 columns used for padding the
>     numbers on display.

Thanks Noam, much appreciation 🙏

I’m sorry to be so blunt, but did anyone actually read this? I’ve read over it several times and cannot understand the writer thought it would make sense to anyone not already intimately familiar with the code. The questions that arise....

- Why does an argument PIXELWISE take a symbol “columns”? 
- Why does PIXELWISE then return characters?
- Still, why would the function not just return the width of the line numbers *with* the 2 columns for padding?

I mean, we’re not dealing with some deeply entrenched internal function where these sorts of mental contortions are required to maintain backwards compatibility, this is a *new* feature.

I understand that this function is also used internally for specific purposes, so it needs to behave a specific way to appease the initial writer of the code. But for the sake of sanity, I strongly suggest either fixing this function or creating a separate “public” function that operates vaguely in line with user expectation for those of us wanting to accommodate display-line-numbers-mode.

bug#29597: 26.0.90; line-number-display-width returns incorrect width

[Eli Zaretskii]

> From: Paul Rankin <hello@paulwrankin.com>
> Cc: eliz@gnu.org
> Date: Fri, 08 Dec 2017 11:44:06 +1000
> 
> I had a look in the source of display-line-numbers.el in the repo but cannot find the definition of line-number-display-width.
> 
> But this is irrelevant, the line number width is clearly 4 characters, lining up with File in the menu bar.
> 
> (string-width "File")
> 4
> 
> It clearly fails to count to four. Adding a docstring that 2 really equals 4 is absurd.

Like I said, it behaves as designed: returns the number of digits used
to display the line number.  (The source is in indent.c.)  If you need
something different for your needs, the function can provide 2 other
related values, controlled by the optional; variable.

All of that is in the doc string on the release branch, and also in
the ELisp manual.

> Please don’t close bugs until they’re actually closed.

I didn't close this bug, so I'm not sure why you are asking to do
something that we always don anyway.

bug#29597: 26.0.90; line-number-display-width returns incorrect width

[Noam Postavsky]

Eli Zaretskii <eliz@gnu.org> writes:

>> Please don’t close bugs until they’re actually closed.
>
> I didn't close this bug, so I'm not sure why you are asking to do
> something that we always don anyway.

Glenn closed the bug.  I think the message sent to the submitter when a
bug is closed has a lot of extra info, so it can be a bit unclear as to
who actually closed the bug.

https://debbugs.gnu.org/cgi/bugreport.cgi?msg=15;bug=29597

bug#29597: 26.0.90; line-number-display-width returns incorrect width

[Eli Zaretskii]

> From: Paul Rankin <hello@paulwrankin.com>
> Cc: 29597@debbugs.gnu.org, eliz@gnu.org
> Date: Fri, 08 Dec 2017 12:32:31 +1000
> 
> - Why does an argument PIXELWISE take a symbol “columns”? 
> - Why does PIXELWISE then return characters?

This is a very widely used size unit in display- and layout-related
APIs in Emacs: the width of the frame's canonical character.  We call
this "columns" because when the face used for display is the default
face, and the font is fixed-pitch, that is what you get.

This is explained in the subsection "Frame Font" in the ELisp manual:

  Each frame has a “default font” which specifies the default character
  size for that frame.  This size is meant when retrieving or changing the
  size of a frame in terms of columns or lines (*note Size Parameters::).
  It is also used when resizing (*note Window Sizes::) or splitting (*note
  Splitting Windows::) windows.

     The terms “line height” and “canonical character height” are
  sometimes used instead of “default character height”.  Similarly, the
  terms “column width” and “canonical character width” are used instead of
  “default character width”.

> - Still, why would the function not just return the width of the line numbers *with* the 2 columns for padding?

Because it returns the number of _digits_ used for line-number
display, omitting the padding.  The reason why this value is returned
is to some extent historical: there are users of that in
display-line-numbers.el.

> I mean, we’re not dealing with some deeply entrenched internal function where these sorts of mental contortions are required to maintain backwards compatibility, this is a *new* feature.

The function is new, but the notion of measuring width in "canonical
columns" is not, it is very old and goes back to Emacs 21, which
introduced support for variable fonts and non-text display elements
(such as images) shown in a window.

> I understand that this function is also used internally for specific purposes, so it needs to behave a specific way to appease the initial writer of the code. But for the sake of sanity, I strongly suggest either fixing this function or creating a separate “public” function that operates vaguely in line with user expectation for those of us wanting to accommodate display-line-numbers-mode.

Sorry, I don't understand what is missing.  If you wanted the value of
4 in your case, you need to call this function like this:

  (line-number-display-width 'columns)

as documented.

bug#29597: 26.0.90; line-number-display-width returns incorrect width

[Noam Postavsky]

On Fri, Dec 8, 2017 at 10:01 AM, Eli Zaretskii <eliz@gnu.org> wrote:

>> I understand that this function is also used internally for specific purposes, so it needs to behave a specific way to appease the initial writer of the code. But for the sake of sanity, I strongly suggest either fixing this function or creating a separate “public” function that operates vaguely in line with user expectation for those of us wanting to accommodate display-line-numbers-mode.
>
> Sorry, I don't understand what is missing.  If you wanted the value of
> 4 in your case, you need to call this function like this:
>
>   (line-number-display-width 'columns)
>
> as documented.

I think the idea would to split that function into 2 simpler ones, e.g.:

(line-number-display-columns) => 4
(line-number-display-width) => 2
(line-number-display-width t) => the equivalent in pixels

bug#29597: 26.0.90; line-number-display-width returns incorrect width

[Eli Zaretskii]

> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Date: Fri, 8 Dec 2017 10:37:09 -0500
> Cc: Paul Rankin <hello@paulwrankin.com>, 29597@debbugs.gnu.org
> 
> I think the idea would to split that function into 2 simpler ones, e.g.:
> 
> (line-number-display-columns) => 4
> (line-number-display-width) => 2
> (line-number-display-width t) => the equivalent in pixels

I don't object to such trivial wrappers, although I'd raise a brow if
I saw something like this in Emacs.  It sounds too trivial to have 2
more symbols in the name-space.  But that's me.

bug#29597: 26.0.90; line-number-display-width returns incorrect width

[Eli Zaretskii]

> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Date: Fri, 8 Dec 2017 10:37:09 -0500
> Cc: Paul Rankin <hello@paulwrankin.com>, 29597@debbugs.gnu.org
> 
> (line-number-display-width) => 2
> (line-number-display-width t) => the equivalent in pixels

Actually, equivalent to what?  If to 2 (not 4), then this value would
be useless, I think.  And if it's equivalent to 4, then it should be
(line-number-display-columns t), right?

bug#29597: 26.0.90; line-number-display-width returns incorrect width

[Noam Postavsky]

On Fri, Dec 8, 2017 at 10:55 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Noam Postavsky <npostavs@users.sourceforge.net>
>> Date: Fri, 8 Dec 2017 10:37:09 -0500
>> Cc: Paul Rankin <hello@paulwrankin.com>, 29597@debbugs.gnu.org
>>
>> (line-number-display-width) => 2
>> (line-number-display-width t) => the equivalent in pixels
>
> Actually, equivalent to what?  If to 2 (not 4), then this value would
> be useless, I think.  And if it's equivalent to 4, then it should be
> (line-number-display-columns t), right?

Oh, yes, I was confused. Because packing so many conditions into the
function interface is confusing (I seem to recall Dmitry raised the
same point when you were introducing the function).

> I don't object to such trivial wrappers, although I'd raise a brow if
> I saw something like this in Emacs.  It sounds too trivial to have 2
> more symbols in the name-space.  But that's me.

Is the symbol space really a scarce resource?

bug#29597: 26.0.90; line-number-display-width returns incorrect width

[Eli Zaretskii]

> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Date: Fri, 8 Dec 2017 11:23:18 -0500
> Cc: Paul Rankin <hello@paulwrankin.com>, 29597@debbugs.gnu.org
> 
> > I don't object to such trivial wrappers, although I'd raise a brow if
> > I saw something like this in Emacs.  It sounds too trivial to have 2
> > more symbols in the name-space.  But that's me.
> 
> Is the symbol space really a scarce resource?

Not really scarce, but do you really feel good with such trivial
wrappers?  Do we have anything similar anywhere else?

Again, I won't fight this, I'm just saying it feels weird to me.

bug#29597: 26.0.90; line-number-display-width returns incorrect width

[Noam Postavsky]

On Fri, Dec 8, 2017 at 11:29 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Noam Postavsky <npostavs@users.sourceforge.net>
>> Date: Fri, 8 Dec 2017 11:23:18 -0500
>> Cc: Paul Rankin <hello@paulwrankin.com>, 29597@debbugs.gnu.org
>>
>> > I don't object to such trivial wrappers, although I'd raise a brow if
>> > I saw something like this in Emacs.  It sounds too trivial to have 2
>> > more symbols in the name-space.  But that's me.
>>
>> Is the symbol space really a scarce resource?
>
> Not really scarce, but do you really feel good with such trivial
> wrappers?  Do we have anything similar anywhere else?

Flag parameters are pretty common in Emacs, but it doesn't make me
feel better about them.
I feel pretty okay about wrappers. For other examples, string-match-p
is a fairly trivial wrapper around string-match.