bug#34513: display-line-numbers in term mode

bug#34513: display-line-numbers in term mode

[Eli Zaretskii]

> From: Ergus <spacibba@aol.com>
> Cc: 34513@debbugs.gnu.org, monnier@iro.umontreal.ca
> Date: Thu 01 Jan 1970 01:00:00 AM CET
> > Window width changes are unrelated to line numbers, right?
> >
> > Is window-size-change-functions what you want?
> 
> Yes they are unrelated, but the issue is the same...

No, the issue is not the same.  When a window is resized, it is
completely redrawn, and as part of that the line numbers are
recalculated and redisplayed.  That's because line-number display is
an inherent part of redisplay.

> Actually on a windows resize the term-line width is not updated
> either. So the new outputs after that assume the same window width. The
> line-number-width is just another side of the same problem where the
> line-width is not recalculated..
> 
> On window increase size the issue is not so annoying (it is not right,
> but it works) but on window decrease size (or line-number-width
> increase) it is worth because the lines adds a \ and break.
> 
> So a right fix maybe should recalculate the line-width more dynamically
> (for example when inserting RET in term mode)

So is window-size-change-functions what you need to use to hook into
the size changes?

bug#34513: display-line-numbers in term mode

[Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

On Tue, Oct 22, 2019 at 08:33:27PM +0300, Eli Zaretskii wrote:
>> From: Ergus <spacibba@aol.com>
>> Cc: 34513@debbugs.gnu.org, monnier@iro.umontreal.ca
>> Date: Thu 01 Jan 1970 01:00:00 AM CET
>> > Window width changes are unrelated to line numbers, right?
>> >
>> > Is window-size-change-functions what you want?
>>
>> Yes they are unrelated, but the issue is the same...
>
>No, the issue is not the same.  When a window is resized, it is
>completely redrawn, and as part of that the line numbers are
>recalculated and redisplayed.  That's because line-number display is
>an inherent part of redisplay.
>
Hi:

I don't have too much time to be looking into this. But actually one of
the issues I had with the Stefan's solution is that sometimes I get:

Error adjusting window size: (wrong-type-argument wholenump 78.0)

Actually very often.

On the other hand, I am not sure that the fix will be needed in
lisp/window.el as it may affect many other things around.

Maybe we need the change in term.el itself as it seems to be the only
one with the issue. Otherwise the fix you made for tabulated-list.el
will be needed as a function in some other place we can access from
term-mode.

Could you please give a look into it?


>> Actually on a windows resize the term-line width is not updated
>> either. So the new outputs after that assume the same window width. The
>> line-number-width is just another side of the same problem where the
>> line-width is not recalculated..
>>
>> On window increase size the issue is not so annoying (it is not right,
>> but it works) but on window decrease size (or line-number-width
>> increase) it is worth because the lines adds a \ and break.
>>
>> So a right fix maybe should recalculate the line-width more dynamically
>> (for example when inserting RET in term mode)
>
>So is window-size-change-functions what you need to use to hook into
>the size changes?

Yes I suppose it is. But I have not clear how to use it :(.

Again,
Could you please give a look into it?
Please.

bug#34513: display-line-numbers in term mode

[Eli Zaretskii]

> Date: Wed, 23 Oct 2019 01:06:05 +0200
> From: Ergus <spacibba@aol.com>
> Cc: 34513@debbugs.gnu.org, monnier@iro.umontreal.ca
> 
> Again,
> Could you please give a look into it?

Look at what? at fixing term.el myself in this case?  Or something
else?

bug#34513: display-line-numbers in term mode

[Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

On Wed, Oct 23, 2019 at 07:21:36PM +0300, Eli Zaretskii wrote:
>> Date: Wed, 23 Oct 2019 01:06:05 +0200
>> From: Ergus <spacibba@aol.com>
>> Cc: 34513@debbugs.gnu.org, monnier@iro.umontreal.ca
>>
>> Again,
>> Could you please give a look into it?
>
>Look at what? at fixing term.el myself in this case?  Or something
>else?

At least if the fix should be in term.el or in window.el like the one
proposed by Stefan.

The lisp side is too high level, and I don't want to waste time
reinventing the wheel.

bug#34513: display-line-numbers in term mode

[Ergus]

display-line-number-mode produces bad term buffers with extra breaks.

When using global-display-line-numbers there are some issues in term
mode because of the extra columns needed by the numbers.

The lines are broken because "tput cols" and $COLUMNS report the total
width of the window, but it does not take into account the 3/4 (or more)
chars taken by the line number. So longer lines need to be broken to fit.

The problem is worst when trying to use for example mocp or similar
curses bases applications. Or in zsh that uses an alternative to
readline to select with tabs.

A workaround is be to disable the numbers in term-mode with a hook (this
hides the problem). But a proper fix (maybe) could be to inform properly the
number of columns to the terminal process.

To reproduce this issue just:
1) open emacs, 
2) enable display-line-numbers-mode
3) M-x term. 

Any curses based program there should expose the issue easily.

I tried with emacs 26.1 and the master branch too.

Regards

bug#34513: display-line-numbers in term mode

[Stefan Monnier]

> display-line-number-mode produces bad term buffers with extra breaks.
> When using global-display-line-numbers there are some issues in term
> mode because of the extra columns needed by the numbers.
>
> The lines are broken because "tput cols" and $COLUMNS report the total
> width of the window, but it does not take into account the 3/4 (or more)
> chars taken by the line number. So longer lines need to be broken to fit.

So it seems that some of the problems are due to the process not being
told the size of the really usable text area.  This is done normally via
window-adjust-process-window-size-function which in turns calls
window-adjust-process-window-size which then calls
window-max-chars-per-line but that function does not take
display-line-numbers-width into account.

So maybe a simple fix is the patch below, but I'm not sure it's always
the right thing to do (and the width won't be automatically updated when
the (line-number-display-width) changes).


        Stefan


PS: Why is the function called `line-number-display-width` when
everything else seems to use the `display-line-number` prefix?  
It makes it more difficult to find.


diff --git a/lisp/window.el b/lisp/window.el
index 07a0f713c4..906e9012fe 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -2132,7 +2132,7 @@ window-max-chars-per-line
   (with-selected-window (window-normalize-window window t)
     (let* ((window-width (window-body-width window t))
 	   (font-width (window-font-width window face))
-	   (ncols (/ window-width font-width)))
+	   (ncols (- (/ window-width font-width) (line-number-display-width))))
       (if (and (display-graphic-p)
 	       overflow-newline-into-fringe
                (not

bug#34513: display-line-numbers in term mode

[Eli Zaretskii]

> From: Stefan Monnier <monnier@IRO.UMontreal.CA>
> Date: Wed, 27 Feb 2019 10:26:04 -0500
> Cc: 34513@debbugs.gnu.org
> 
> PS: Why is the function called `line-number-display-width` when
> everything else seems to use the `display-line-number` prefix?  

Because display-line-numbers-line-number-display-width is a mouthful
that I couldn't live with, and also repeats 3 words twice each.

> -	   (ncols (/ window-width font-width)))
> +	   (ncols (- (/ window-width font-width) (line-number-display-width))))

I think you want (line-number-display-width 'columns), since this
wants the canonical column units, right?

bug#34513: display-line-numbers in term mode

[Stefan Monnier]

>> PS: Why is the function called `line-number-display-width` when
>> everything else seems to use the `display-line-number` prefix?
> Because display-line-numbers-line-number-display-width is a mouthful
> that I couldn't live with, and also repeats 3 words twice each.

That would be awful, indeed.

How 'bout display-line-numbers-width or display-line-numbers-current-width?

>> -	   (ncols (/ window-width font-width)))
>> +	   (ncols (- (/ window-width font-width) (line-number-display-width))))
> I think you want (line-number-display-width 'columns), since this
> wants the canonical column units, right?

Could be, I don't know.  I don't know that all callers of
window-max-chars-per-line want that either.


        Stefan

bug#34513: display-line-numbers in term mode

[Eli Zaretskii]

> From: Stefan Monnier <monnier@IRO.UMontreal.CA>
> Cc: spacibba@aol.com, 34513@debbugs.gnu.org
> Date: Wed, 27 Feb 2019 11:40:04 -0500
> 
> How 'bout display-line-numbers-width or display-line-numbers-current-width?

I think I considered those at the time, but found them not descriptive
enough.

And please remember that originally the code was entirely in C, so
there was no "package name" to use as prefix.  display-line-numbers.el
and display-line-numbers-mode were born much later.

> >> -	   (ncols (/ window-width font-width)))
> >> +	   (ncols (- (/ window-width font-width) (line-number-display-width))))
> > I think you want (line-number-display-width 'columns), since this
> > wants the canonical column units, right?
> 
> Could be, I don't know.  I don't know that all callers of
> window-max-chars-per-line want that either.

I hope someone does know.

bug#34513: display-line-numbers in term mode

[Stefan Monnier]

>> How 'bout display-line-numbers-width or display-line-numbers-current-width?
> I think I considered those at the time, but found them not descriptive
> enough.

And how 'bout now?

> And please remember that originally the code was entirely in C, so
> there was no "package name" to use as prefix.

Oh, of course.  But I think in the current context, it would be good to
bring that variable into the "display-line-numbers" prefix.

At first, TAB completion gave me the impression that there was simply no
way to know from Elisp the actual width, because the list of completions
seemed long enough to have the appearance of exhaustiveness.

>> >> -	   (ncols (/ window-width font-width)))
>> >> +	   (ncols (- (/ window-width font-width) (line-number-display-width))))
>> > I think you want (line-number-display-width 'columns), since this
>> > wants the canonical column units, right?
>> Could be, I don't know.  I don't know that all callers of
>> window-max-chars-per-line want that either.
> I hope someone does know.

So do I.  Martin maybe?


        Stefan

bug#34513: display-line-numbers in term mode

[Eli Zaretskii]

> From: Stefan Monnier <monnier@IRO.UMontreal.CA>
> Cc: spacibba@aol.com, 34513@debbugs.gnu.org
> Date: Wed, 27 Feb 2019 12:21:26 -0500
> 
> >> How 'bout display-line-numbers-width or display-line-numbers-current-width?
> > I think I considered those at the time, but found them not descriptive
> > enough.
> 
> And how 'bout now?

Same.

My problem was, and is, that the name should include "display-width",
since that's what the function provides.  And the problem, of course,
is that "display-line-numbers-" already includes "display" and
"line-number", so repeating any of that produces problematic names.

> At first, TAB completion gave me the impression that there was simply no
> way to know from Elisp the actual width, because the list of completions
> seemed long enough to have the appearance of exhaustiveness.

Maybe just mentioning the variable in the doc string of
display-line-numbers-mode would be "good enough".

bug#34513: display-line-numbers in term mode

[Drew Adams]

> My problem was, and is, that the name should include "display-width",
> since that's what the function provides.  And the problem, of course,
> is that "display-line-numbers-" already includes "display" and
> "line-number", so repeating any of that produces problematic names.

Not following this thread.  Feel free to ignore.

Why does the name need to include "display-width"?
What's unclear about, say, `line-number-width'?

bug#34513: display-line-numbers in term mode

[Eli Zaretskii]

> Date: Wed, 27 Feb 2019 10:00:25 -0800 (PST)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: 34513@debbugs.gnu.org, spacibba@aol.com
> 
> What's unclear about, say, `line-number-width'?

It's not unclear, it's wrong.

And even if we disregard the fact that it's wrong, it still includes
"line-number", which is why "display-line-numbers-number-width" gives
me nausea.

bug#34513: display-line-numbers in term mode

[Drew Adams]

> > What's unclear about, say, `line-number-width'?
> 
> It's not unclear, it's wrong.

I didn't realize that this is a minor-mode global
function.

Emacs convention says that the names of global
symbols (such as this) should start with either
the mode name OR an abbreviation.

The mode name `display-line-numbers' (mode) is
long enough to deserve an abbreviation for use
as prefix.  Please consider coming up with one.

Maybe something like `linenum-' or `line-number-'.

In that case, this function could be called just
`linenum-width' or `linenum-number-width'.

bug#34513: display-line-numbers in term mode

[Lars Ingebrigtsen]

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

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

> So maybe a simple fix is the patch below, but I'm not sure it's always
> the right thing to do (and the width won't be automatically updated when
> the (line-number-display-width) changes).

[...]

> -	   (ncols (/ window-width font-width)))
> +	   (ncols (- (/ window-width font-width) (line-number-display-width))))

I tried this patch in Emacs 28, and it didn't seem to help much.

To test

emacs -Q
M-x global-display-line-numbers-mode RET
M-x term RET RET
emacs -Q -nw


[-- Attachment #2: Type: image/png, Size: 71413 bytes --]

[-- Attachment #3: Type: text/plain, Size: 207 bytes --]


Note wrapped mode line of the inner Emacs.

(This works fine if display-line-numbers-mode is not on.)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#34513: display-line-numbers in term mode

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Date: Sun, 20 Sep 2020 20:30:33 +0200
> Cc: 34513@debbugs.gnu.org, Ergus <spacibba@aol.com>
> 
> > -	   (ncols (/ window-width font-width)))
> > +	   (ncols (- (/ window-width font-width) (line-number-display-width))))
> 
> I tried this patch in Emacs 28, and it didn't seem to help much.

Not even if you call line-number-display-width with arg of 'columns'?

Does it help to set COLUMNS in the environment to something like 76
before starting "M-x term"?  If it does, that would at least tell us
the columns are the cause of the problem.

bug#34513: display-line-numbers in term mode

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> Not even if you call line-number-display-width with arg of 'columns'?

Yes, this works -- the Emacs-in-Emacs displays perfectly with this patch:

diff --git a/lisp/window.el b/lisp/window.el
index f1ee87aad2..72957d846a 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -2172,7 +2172,8 @@ window-max-chars-per-line
   (with-selected-window (window-normalize-window window t)
     (let* ((window-width (window-body-width window t))
 	   (font-width (window-font-width window face))
-	   (ncols (/ window-width font-width)))
+	   (ncols (- (/ window-width font-width)
+                     (line-number-display-width 'columns))))
       (if (and (display-graphic-p)
 	       overflow-newline-into-fringe
                (not


-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#34513: display-line-numbers in term mode

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: monnier@IRO.UMontreal.CA,  34513@debbugs.gnu.org,  spacibba@aol.com
> Date: Sun, 20 Sep 2020 21:56:54 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Not even if you call line-number-display-width with arg of 'columns'?
> 
> Yes, this works -- the Emacs-in-Emacs displays perfectly with this patch:
> 
> diff --git a/lisp/window.el b/lisp/window.el
> index f1ee87aad2..72957d846a 100644
> --- a/lisp/window.el
> +++ b/lisp/window.el
> @@ -2172,7 +2172,8 @@ window-max-chars-per-line
>    (with-selected-window (window-normalize-window window t)
>      (let* ((window-width (window-body-width window t))
>  	   (font-width (window-font-width window face))
> -	   (ncols (/ window-width font-width)))
> +	   (ncols (- (/ window-width font-width)
> +                     (line-number-display-width 'columns))))
>        (if (and (display-graphic-p)
>  	       overflow-newline-into-fringe
>                 (not

Great, then please install, and thanks.

bug#34513: display-line-numbers in term mode

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> Great, then please install, and thanks.

OK; applied to Emacs 28.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#34513: display-line-numbers in term mode

[Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Hi:

I have tried the quick solution Stefan proposed to this but there are
still limitations when the number column width change and some
others when resizing the window. Does anyone has a way to improve
this?

I think that this must be simple enough to fix quickly in order to not
pass this to emacs 27.

On the other hand I wanted to ask if new changes like dfci must be
disabled in term-mode by default or we let that configuration to the
user?

bug#34513: display-line-numbers in term mode

[Eli Zaretskii]

> Date: Mon, 21 Oct 2019 00:54:46 +0200
> From: Ergus <spacibba@aol.com>
> Cc: Eli Zaretskii <eliz@gnu.org>, Stefan Monnier <monnier@iro.umontreal.ca>
> 
> I have tried the quick solution Stefan proposed to this but there are
> still limitations when the number column width change and some
> others when resizing the window. Does anyone has a way to improve
> this?

You can see the various solutions in tabulated-list.el.  I wrote them
because features like package.el need to react to the changing width
of the line numbers.

In addition to the change in the line numbers display width due to
scrolling, there are changes due to resizing the default font.

> On the other hand I wanted to ask if new changes like dfci must be
> disabled in term-mode by default or we let that configuration to the
> user?

This is a different issue, unrelated to this bug report.  Let's
discuss it separately, probably on emacs-devel.

Thanks.

bug#34513: display-line-numbers in term mode

[Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

On Mon, Oct 21, 2019 at 09:51:47AM +0300, Eli Zaretskii wrote:
>> Date: Mon, 21 Oct 2019 00:54:46 +0200
>> From: Ergus <spacibba@aol.com>
>> Cc: Eli Zaretskii <eliz@gnu.org>, Stefan Monnier <monnier@iro.umontreal.ca>
>>
>> I have tried the quick solution Stefan proposed to this but there are
>> still limitations when the number column width change and some
>> others when resizing the window. Does anyone has a way to improve
>> this?
>
>You can see the various solutions in tabulated-list.el.  I wrote them
>because features like package.el need to react to the changing width
>of the line numbers.
>
>In addition to the change in the line numbers display width due to
>scrolling, there are changes due to resizing the default font.
>
Hi Eli:

I see that, but what about when the window width changes? Are there any
hook we ca use?

>> On the other hand I wanted to ask if new changes like dfci must be
>> disabled in term-mode by default or we let that configuration to the
>> user?
>
>This is a different issue, unrelated to this bug report.  Let's
>discuss it separately, probably on emacs-devel.
>
>Thanks.

bug#34513: display-line-numbers in term mode

[Eli Zaretskii]

> Date: Mon, 21 Oct 2019 23:44:11 +0200
> From: Ergus <spacibba@aol.com>
> Cc: 34513@debbugs.gnu.org, monnier@iro.umontreal.ca
> 
> >You can see the various solutions in tabulated-list.el.  I wrote them
> >because features like package.el need to react to the changing width
> >of the line numbers.
> >
> >In addition to the change in the line numbers display width due to
> >scrolling, there are changes due to resizing the default font.
> >
> Hi Eli:
> 
> I see that, but what about when the window width changes? Are there any
> hook we ca use?

Window width changes are unrelated to line numbers, right?

Is window-size-change-functions what you want?