bug#37564: [PATCH] don't export LINES and COLUMNS env vars in term to fix ncurses applications

bug#37564: [PATCH] don't export LINES and COLUMNS env vars in term to fix ncurses applications

[Matthew Leach]

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

Hi all,

The attached patch removes the exporting of the LINES and COLUMNS
environment variables in term-mode.  Exporting these variables causes
issues for ncurses applications.  For example, when running:

emacs -Q
M-x term
<ret> (to select /bin/bash)
htop (to run htop)

and resizing the window (especially making it smaller) can make the
program impossible to read.

The ncurses code uses an ioctl() initially to get the current size[1]
which returns a correct result. However, since we exported the above
environment variables these values are discarded in favour of the (now
stale) values of LINES and COLUMNS. Emacs makes no attempt to update
these variables when the window is resized. Therefore, ncurses assumes
the window size is the same when it has actually changed.

Note that executing:

$ echo $LINES

from a bash shell actually doesn't get the LINES environment variable as
can be seen with:

$ printenv | grep LINES

Whenever $LINES is accessed, bash translates this into an ioctl() and
returns the result. Therefore, removing these variables shouldn't
prevent any shell scripts from accessing these variables.

[1]: ncurses/ncurses/tinfo/lib_setup.c (_nc_get_screensize)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-don-t-export-LINES-and-COLUMNS-env-vars-to-term.patch --]
[-- Type: text/x-patch, Size: 1015 bytes --]

From 20b1adff1c32cfeeba1cf3101b05d5c76037206d Mon Sep 17 00:00:00 2001
From: Matthew Leach <matthew@mattleach.net>
Date: Mon, 30 Sep 2019 20:35:54 +0100
Subject: [PATCH] don't export LINES and COLUMNS env vars to term

* lisp/term.el (term-exec-1): Remove the exporting of the LINES and
COLUMNS environment variables to the terminal process.
---
 lisp/term.el | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/lisp/term.el b/lisp/term.el
index 66ae470239a..e309bd802d5 100644
--- a/lisp/term.el
+++ b/lisp/term.el
@@ -1550,9 +1550,7 @@ term-exec-1
 	   (format term-termcap-format "TERMCAP="
 		   term-term-name term-height term-width)
 
-	   (format "INSIDE_EMACS=%s,term:%s" emacs-version term-protocol-version)
-	   (format "LINES=%d" term-height)
-	   (format "COLUMNS=%d" term-width))
+	   (format "INSIDE_EMACS=%s,term:%s" emacs-version term-protocol-version))
 	  process-environment))
 	(process-connection-type t)
 	;; We should suppress conversion of end-of-line format.
-- 
2.23.0


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

-- 
Matt

bug#37564: [PATCH] don't export LINES and COLUMNS env vars in term to fix ncurses applications

[Eli Zaretskii]

> From: Matthew Leach <matthew@mattleach.net>
> Date: Mon, 30 Sep 2019 20:59:04 +0100
> 
> The attached patch removes the exporting of the LINES and COLUMNS
> environment variables in term-mode.  Exporting these variables causes
> issues for ncurses applications.  For example, when running:
> 
> emacs -Q
> M-x term
> <ret> (to select /bin/bash)
> htop (to run htop)
> 
> and resizing the window (especially making it smaller) can make the
> program impossible to read.

Thanks, but I don't think we can make this change unconditionally,
because not all applications that heed LINES and COLUMNS use ncurses.
I wonder if we can do better than just providing a defcustom.

bug#37564: [PATCH] don't export LINES and COLUMNS env vars in term to fix ncurses applications

[Matthew Leach]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Matthew Leach <matthew@mattleach.net>
>> Date: Mon, 30 Sep 2019 20:59:04 +0100
>> 
>> The attached patch removes the exporting of the LINES and COLUMNS
>> environment variables in term-mode.  Exporting these variables causes
>> issues for ncurses applications.  For example, when running:
>> 
>> emacs -Q
>> M-x term
>> <ret> (to select /bin/bash)
>> htop (to run htop)
>> 
>> and resizing the window (especially making it smaller) can make the
>> program impossible to read.
>
> Thanks, but I don't think we can make this change unconditionally,
> because not all applications that heed LINES and COLUMNS use ncurses.

I'm curious as to which programs you are referring? AFAIK, if a program
tries to read the LINES and COLUMNS environment variables, using
`getenv()', they don't exist. Running 'echo $LINES' on a bash terminal
seems to actually do an ioctl to obtain the value of LINES.

Nevertheless, if a program does read the LINES and COLUMNS variables,
these values will be wrong if the window has been resized (try and
compile the attached C snippet and run in term mode while resizing the
window). Should that be considered as a separate bug?

> I wonder if we can do better than just providing a defcustom.

I could make this a defcustom if that would get the patch in?

Thanks,
-- 
Matt

[-- Attachment #2: winch2.c --]
[-- Type: text/plain, Size: 370 bytes --]

#include <stdio.h>
#include <signal.h>
#include <unistd.h>
#include <sys/ioctl.h>

void handle_sig()
{
    struct winsize w;
    ioctl(STDOUT_FILENO, TIOCGWINSZ, &w);

    printf("getenv: %s %s\n", getenv("LINES"), getenv("COLUMNS"));
    printf("ioctl:  %d %d\n", w.ws_row, w.ws_col);
}

int main()
{
    signal(SIGWINCH, handle_sig);
    fgetc(stdin);
    return 0;
}

bug#37564: [PATCH] don't export LINES and COLUMNS env vars in term to fix ncurses applications

[Eli Zaretskii]

> From: Matthew Leach <matthew@mattleach.net>
> Cc: 37564@debbugs.gnu.org
> Date: Tue, 01 Oct 2019 16:47:35 +0100
> 
> > Thanks, but I don't think we can make this change unconditionally,
> > because not all applications that heed LINES and COLUMNS use ncurses.
> 
> I'm curious as to which programs you are referring? AFAIK, if a program
> tries to read the LINES and COLUMNS environment variables, using
> `getenv()', they don't exist.

What makes you say that?  Emacs exports these variables into the
environment that is passed to child subprocesses, so those
subprocesses will definitely see them using getenv.

> Nevertheless, if a program does read the LINES and COLUMNS variables,
> these values will be wrong if the window has been resized (try and
> compile the attached C snippet and run in term mode while resizing the
> window). Should that be considered as a separate bug?

We nowadays have window-adjust-process-window-size-function to support
that, and term.el is using that.  So why isn't it working for you?
Maybe you are running a version of Emacs that's too old (AFAICT, you
didn't say which one)?

bug#37564: [PATCH] don't export LINES and COLUMNS env vars in term to fix ncurses applications

[Matthew Leach]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Matthew Leach <matthew@mattleach.net>
>> Cc: 37564@debbugs.gnu.org
>> Date: Tue, 01 Oct 2019 16:47:35 +0100
>> 
>> > Thanks, but I don't think we can make this change unconditionally,
>> > because not all applications that heed LINES and COLUMNS use ncurses.
>> 
>> I'm curious as to which programs you are referring? AFAIK, if a program
>> tries to read the LINES and COLUMNS environment variables, using
>> `getenv()', they don't exist.
>
> What makes you say that? 

If I run the C program in my previous post in an xterm and resize I get:

getenv: (null) (null)
ioctl:  24 80
getenv: (null) (null)
ioctl:  42 169

You can see that doing getenv() on LINES and COLUMNS returns a null
pointer.  I've tested this same behaviour on gnome-terminal and numerous
other terminal emulators.

> Emacs exports these variables into the environment that is passed to
> child subprocesses, so those subprocesses will definitely see them
> using getenv.

I'm no expert on how these things work but it doesn't appear to be the
way that it's done, on my machine anyway.  Perhaps there are programs
that uses these variables on MacOS or Windows?

>> Nevertheless, if a program does read the LINES and COLUMNS variables,
>> these values will be wrong if the window has been resized (try and
>> compile the attached C snippet and run in term mode while resizing the
>> window). Should that be considered as a separate bug?
>
> We nowadays have window-adjust-process-window-size-function to support
> that, and term.el is using that.  So why isn't it working for you?
> Maybe you are running a version of Emacs that's too old (AFAICT, you
> didn't say which one)?

I'm testing this on the latest Emacs Git version. I can see that
window-adjust-process-window-size-function is used in term.el however,
this doesn't appear to update the environment variables exported to the
terminal process. If I run the same program with term on Emacs git and
resize I get:

getenv: 31 80
ioctl:  30 89
getenv: 31 80
ioctl:  30 90
getenv: 31 80
ioctl:  30 91
getenv: 31 80
ioctl:  31 92
getenv: 31 80
ioctl:  31 93
getenv: 31 80
ioctl:  31 94
...

Notice that the values returned by getenv don't change.

Given that this functionality is broken, I'm suggesting that we don't
export the variables. This will make ncurses fall back to using ioctls
for window size inspection. Another way to fix this would be to somehow
update the environment variables that are exported to the process. What
are your thoughts?

Thanks,
-- 
Matt

bug#37564: [PATCH] don't export LINES and COLUMNS env vars in term to fix ncurses applications

[Eli Zaretskii]

> From: Matthew Leach <matthew@mattleach.net>
> Cc: 37564@debbugs.gnu.org
> Date: Tue, 01 Oct 2019 19:33:39 +0100
> 
> >> I'm curious as to which programs you are referring? AFAIK, if a program
> >> tries to read the LINES and COLUMNS environment variables, using
> >> `getenv()', they don't exist.
> >
> > What makes you say that? 
> 
> If I run the C program in my previous post in an xterm and resize I get:
> 
> getenv: (null) (null)
> ioctl:  24 80
> getenv: (null) (null)
> ioctl:  42 169
> 
> You can see that doing getenv() on LINES and COLUMNS returns a null
> pointer.  I've tested this same behaviour on gnome-terminal and numerous
> other terminal emulators.

I simply tried "env | fgrep LINES" and I do see the variable, although
it wasn't there before I launched Emacs.  'env' is just another
program accessing the environment, right?  I have no idea why your
test program doesn't see that, but then I'm far from being an expert
on these issues.

> > Emacs exports these variables into the environment that is passed to
> > child subprocesses, so those subprocesses will definitely see them
> > using getenv.
> 
> I'm no expert on how these things work but it doesn't appear to be the
> way that it's done, on my machine anyway.  Perhaps there are programs
> that uses these variables on MacOS or Windows?

I tried the above on a Trisquel GNU/Windows machine, FWIW.

> I'm testing this on the latest Emacs Git version. I can see that
> window-adjust-process-window-size-function is used in term.el however,
> this doesn't appear to update the environment variables exported to the
> terminal process.

AFAIR, it's supposed to send the corresponding ioctl command, in
addition to setting LINES and COLUMNS.

> If I run the same program with term on Emacs git and
> resize I get:
> 
> getenv: 31 80
> ioctl:  30 89
> getenv: 31 80
> ioctl:  30 90
> getenv: 31 80
> ioctl:  30 91
> getenv: 31 80
> ioctl:  31 92
> getenv: 31 80
> ioctl:  31 93
> getenv: 31 80
> ioctl:  31 94
> ...
> 
> Notice that the values returned by getenv don't change.

Again, the "env | fgrep LINES" method does show the change when I
resize the window on the machine I tried this, so I'm unsure why you
don't see it.

> Given that this functionality is broken, I'm suggesting that we don't
> export the variables.

I don't agree that it's broken; it isn't for me.  I think someone who
understands more than I do about this stuff should chime in and
explain why we see such different results.

bug#37564: [PATCH] don't export LINES and COLUMNS env vars in term to fix ncurses applications

[Eli Zaretskii]

> Date: Tue, 01 Oct 2019 21:58:25 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 37564@debbugs.gnu.org
> 
> I tried the above on a Trisquel GNU/Windows machine, FWIW.

I meant GNU/Linux, of course.  Sorry...

bug#37564: [PATCH] don't export LINES and COLUMNS env vars in term to fix ncurses applications

[Matthew Leach]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Matthew Leach <matthew@mattleach.net>
>> Cc: 37564@debbugs.gnu.org
>> Date: Tue, 01 Oct 2019 19:33:39 +0100
>> 
>> >> I'm curious as to which programs you are referring? AFAIK, if a program
>> >> tries to read the LINES and COLUMNS environment variables, using
>> >> `getenv()', they don't exist.
>> >
>> > What makes you say that? 
>> 
>> If I run the C program in my previous post in an xterm and resize I get:
>> 
>> getenv: (null) (null)
>> ioctl:  24 80
>> getenv: (null) (null)
>> ioctl:  42 169
>> 
>> You can see that doing getenv() on LINES and COLUMNS returns a null
>> pointer.  I've tested this same behaviour on gnome-terminal and numerous
>> other terminal emulators.
>
> I simply tried "env | fgrep LINES" and I do see the variable, although
> it wasn't there before I launched Emacs.

I would expect the variable to be there when inside Emacs as it has
exported it.  You should find that it won't exist if ran outside Emacs,
for example in xterm.

> 'env' is just another program accessing the environment, right? I have
> no idea why your test program doesn't see that, but then I'm far from
> being an expert on these issues.

Indeed, it does see the variables if I run inside Emacs.  If I run it
outside then they are not there, on my machine anyway.

>> > Emacs exports these variables into the environment that is passed to
>> > child subprocesses, so those subprocesses will definitely see them
>> > using getenv.
>> 
>> I'm no expert on how these things work but it doesn't appear to be the
>> way that it's done, on my machine anyway.  Perhaps there are programs
>> that uses these variables on MacOS or Windows?
>
> I tried the above on a Trisquel GNU/Windows machine, FWIW.

I'm running on Arch GNU/Linux this end.

>> I'm testing this on the latest Emacs Git version. I can see that
>> window-adjust-process-window-size-function is used in term.el however,
>> this doesn't appear to update the environment variables exported to the
>> terminal process.
>
> AFAIR, it's supposed to send the corresponding ioctl command, in
> addition to setting LINES and COLUMNS.
>
>> If I run the same program with term on Emacs git and
>> resize I get:
>> 
>> getenv: 31 80
>> ioctl:  30 89
>> getenv: 31 80
>> ioctl:  30 90
>> getenv: 31 80
>> ioctl:  30 91
>> getenv: 31 80
>> ioctl:  31 92
>> getenv: 31 80
>> ioctl:  31 93
>> getenv: 31 80
>> ioctl:  31 94
>> ...
>> 
>> Notice that the values returned by getenv don't change.
>
> Again, the "env | fgrep LINES" method does show the change when I
> resize the window on the machine I tried this, so I'm unsure why you
> don't see it.

Ah, indeed if I try and resize and print the variable it has updated:

matthew@hopton ~ $ env | grep -i lines
LINES=30
matthew@hopton ~ $ env | grep -i lines
LINES=12

I think the issue is that the environment can't be while a process is
running; the environment variables are fixed when the program has
started. For a ncurses application this presents a problem as LINES and
COLUMNS can't be updated when the window size is changed and the program
handles a SIGWINCH. You can see that on the output above.

>> Given that this functionality is broken, I'm suggesting that we don't
>> export the variables.
>
> I don't agree that it's broken; it isn't for me.  I think someone who
> understands more than I do about this stuff should chime in and
> explain why we see such different results.

I think the above explains what's going on. Perhaps that's why terminal
emulators don't export LINES and COLUMNS to programs so an ioctl() is used
instead?
-- 
Matt

bug#37564: [PATCH] don't export LINES and COLUMNS env vars in term to fix ncurses applications

[Eli Zaretskii]

> From: Matthew Leach <matthew@mattleach.net>
> Cc: 37564@debbugs.gnu.org
> Date: Tue, 01 Oct 2019 20:24:14 +0100
> 
> > I simply tried "env | fgrep LINES" and I do see the variable, although
> > it wasn't there before I launched Emacs.
> 
> I would expect the variable to be there when inside Emacs as it has
> exported it.  You should find that it won't exist if ran outside Emacs,
> for example in xterm.

Sorry, I'm confused.  Are we talking about running "M-x term" inside
Emacs, or are we talking about something else?  The subject of the bug
report says "term", so I assumed you mean term-mode.

> Ah, indeed if I try and resize and print the variable it has updated:
> 
> matthew@hopton ~ $ env | grep -i lines
> LINES=30
> matthew@hopton ~ $ env | grep -i lines
> LINES=12
> 
> I think the issue is that the environment can't be while a process is
> running; the environment variables are fixed when the program has
> started. For a ncurses application this presents a problem as LINES and
> COLUMNS can't be updated when the window size is changed and the program
> handles a SIGWINCH. You can see that on the output above.

While the process is running, Emacs sends ioctl commands to
communicate the window size changes.

bug#37564: [PATCH] don't export LINES and COLUMNS env vars in term to fix ncurses applications

[Glenn Morris]

I think the point is that no terminal emulator / shell combination
actually exports LINES and COLUMNS as environment variables, except for
Emacs term.el. So no application can be relying on the LINES and COLUMNS
environment variables (since there aren't any applications specifically
for use inside Emacs's term). So term.el should stop setting them, since
it actually causes problems.

bug#37564: [PATCH] don't export LINES and COLUMNS env vars in term to fix ncurses applications

[Matthew Leach]

Glenn Morris <rgm@gnu.org> writes:

> I think the point is that no terminal emulator / shell combination
> actually exports LINES and COLUMNS as environment variables, except for
> Emacs term.el. So no application can be relying on the LINES and COLUMNS
> environment variables (since there aren't any applications specifically
> for use inside Emacs's term). So term.el should stop setting them, since
> it actually causes problems.

Exactly that.  By exporting these variables it's causing more problems
than it solves.
-- 
Matt

bug#37564: [PATCH] don't export LINES and COLUMNS env vars in term to fix ncurses applications

[Stefan Kangas]

Matthew Leach <matthew@mattleach.net> writes:

> Glenn Morris <rgm@gnu.org> writes:
>
>> I think the point is that no terminal emulator / shell combination
>> actually exports LINES and COLUMNS as environment variables, except for
>> Emacs term.el. So no application can be relying on the LINES and COLUMNS
>> environment variables (since there aren't any applications specifically
>> for use inside Emacs's term). So term.el should stop setting them, since
>> it actually causes problems.
>
> Exactly that.  By exporting these variables it's causing more problems
> than it solves.

That sounds reasonable to me.  Should we go ahead and install the
original patch on the master branch then, or is there more to discuss?

Best regards,
Stefan Kangas

bug#37564: [PATCH] don't export LINES and COLUMNS env vars in term to fix ncurses applications

[Eli Zaretskii]

> From: Stefan Kangas <stefan@marxist.se>
> Cc: Glenn Morris <rgm@gnu.org>,  Eli Zaretskii <eliz@gnu.org>,
>   37564@debbugs.gnu.org
> Date: Mon, 20 Jan 2020 20:10:49 +0100
> 
> Matthew Leach <matthew@mattleach.net> writes:
> 
> > Glenn Morris <rgm@gnu.org> writes:
> >
> >> I think the point is that no terminal emulator / shell combination
> >> actually exports LINES and COLUMNS as environment variables, except for
> >> Emacs term.el. So no application can be relying on the LINES and COLUMNS
> >> environment variables (since there aren't any applications specifically
> >> for use inside Emacs's term). So term.el should stop setting them, since
> >> it actually causes problems.
> >
> > Exactly that.  By exporting these variables it's causing more problems
> > than it solves.
> 
> That sounds reasonable to me.  Should we go ahead and install the
> original patch on the master branch then, or is there more to discuss?

Sorry, but I need more than just assertions to convince me.  I have
yet to see an application that doesn't cater to LINES and COLUMNS.
Heck, Emacs itself does!  It is true that these variables nowadays are
mostly kept for the users, but that doesn't yet mean Emacs cannot use
them to affect the programs it runs.  As for "causing more problems
than it solves", I don't think I saw any evidence for that: even the
one use case which started this bug report was later shown to behave
as expected.  Or maybe I was just confused, in which case I'd
appreciate a clear and concise description of how these variables are
harmful for ncurses programs.

bug#37564: [PATCH] don't export LINES and COLUMNS env vars in term to fix ncurses applications

[Glenn Morris]

The report seems clear to me.

No other terminal emulator exports LINES and COLUMNS as environment variables.
Applications use ioctl to get the size.
   -> Therefore setting LINES and COLUMNS is pointless.
If LINES/COLUMNS are set, they override ioctl.
You can't update the environment of an already running process.
Therefore the presence of LINES/COLUMNS in the initial environment
of a process spawned from Emacs means it will fail to react to resizing.
 Therefore exporting LINES and COLUMNS is harmful.
(It's irrelevant if Emacs changes LINES and COLUMNS for new processes.)

So don't export LINES and COLUMNS.

I can't explain it any better.

bug#37564: [PATCH] don't export LINES and COLUMNS env vars in term to fix ncurses applications

[Eli Zaretskii]

> From: Glenn Morris <rgm@gnu.org>
> Cc: Stefan Kangas <stefan@marxist.se>,  matthew@mattleach.net,  37564@debbugs.gnu.org
> Date: Mon, 20 Jan 2020 19:58:53 -0500
> 
> 
> No other terminal emulator exports LINES and COLUMNS as environment variables.

Maybe so, but that in itself doesn't yet mean Emacs cannot do that, if
we decide it's useful.

> Applications use ioctl to get the size.
>    -> Therefore setting LINES and COLUMNS is pointless.
> If LINES/COLUMNS are set, they override ioctl.

Is the last bit a generally accepted paradigm?  I could easily justify
the contrary preference.

And then there might be applications that don't use ioctl.

> You can't update the environment of an already running process.
> Therefore the presence of LINES/COLUMNS in the initial environment
> of a process spawned from Emacs means it will fail to react to resizing.
>  Therefore exporting LINES and COLUMNS is harmful.

Only if the application uses the variables in preference to ioctl.

> So don't export LINES and COLUMNS.

I agreed at the very beginning of the discussion to make the export
optional.  I don't think we can safely remove that unconditionally,
not yet.

bug#37564: [PATCH] don't export LINES and COLUMNS env vars in term to fix ncurses applications

[Matthew Leach]

Eli Zaretskii <eliz@gnu.org> writes:


>> Applications use ioctl to get the size.
>>    -> Therefore setting LINES and COLUMNS is pointless.
>> If LINES/COLUMNS are set, they override ioctl.
>
> Is the last bit a generally accepted paradigm? 

I believe so, yes.  At least, that is the case with ncurses.
-- 
Matt

bug#37564: [PATCH] don't export LINES and COLUMNS env vars in term to fix ncurses applications

[Glenn Morris]

PS for an example of the effect, one can use some of the programs in
the ncurses-examples from
https://invisible-island.net/ncurses/ncurses-examples.html

eg "worm".

If I run it in an xfce4-terminal, then maximise the terminal, the worms
spread over the whole screen.
If I "export LINES" and COLUMNS beforehand, then the worms don't expand
beyond the original terminal size.

The same effect is visible in Emacs's M-x term with and without the
LINES/COLUMNS variables.

bug#37564: [PATCH] don't export LINES and COLUMNS env vars in term to fix ncurses applications

[Eli Zaretskii]

> From: Glenn Morris <rgm@gnu.org>
> Cc: Stefan Kangas <stefan@marxist.se>,  matthew@mattleach.net,  37564@debbugs.gnu.org
> Date: Tue, 21 Jan 2020 22:38:10 -0500
> 
> 
> PS for an example of the effect, one can use some of the programs in
> the ncurses-examples from
> https://invisible-island.net/ncurses/ncurses-examples.html
> 
> eg "worm".

I understand that applications based on ncurses are affected, but what
about those which aren't based on ncurses?

bug#37564: [PATCH] don't export LINES and COLUMNS env vars in term to fix ncurses applications

[Glenn Morris]

Eli Zaretskii wrote:

> I understand that applications based on ncurses are affected, but what
> about those which aren't based on ncurses?

I can only repeat my comments from https://debbugs.gnu.org/37564#32

    no terminal emulator / shell combination actually exports LINES and
    COLUMNS as environment variables, except for Emacs term.el. So no
    application can be relying on the LINES and COLUMNS environment
    variables (since there aren't any applications specifically for use
    inside Emacs's term).

bug#37564: [PATCH] don't export LINES and COLUMNS env vars in term to fix ncurses applications

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

>> PS for an example of the effect, one can use some of the programs in
>> the ncurses-examples from
>> https://invisible-island.net/ncurses/ncurses-examples.html
>> 
>> eg "worm".
>
> I understand that applications based on ncurses are affected, but what
> about those which aren't based on ncurses?

I think the rough consensus here was that adding LINES/COLUMNS is
usually the wrong thing to do, but Eli worries that this might break
something.  So I've applied Matthew's patch, but also added a defcustom
to control this.

I've defaulted the variable to "don't add the variables", but if this
leads to problems during the Emacs 28.1 cycle, we can flip it to "on"
later.

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

bug#37564: [PATCH] don't export LINES and COLUMNS env vars in term to fix ncurses applications

[Stefan Kangas]

Glenn Morris <rgm@gnu.org> writes:

> PS for an example of the effect, one can use some of the programs in
> the ncurses-examples from
> https://invisible-island.net/ncurses/ncurses-examples.html
>
> eg "worm".
>
> If I run it in an xfce4-terminal, then maximise the terminal, the worms
> spread over the whole screen.
> If I "export LINES" and COLUMNS beforehand, then the worms don't expand
> beyond the original terminal size.
>
> The same effect is visible in Emacs's M-x term with and without the
> LINES/COLUMNS variables.

BTW, we also have Bug#11432.  Not sure if that should be merged with
this one.

Best regards,
Stefan Kangas

bug#37564: [PATCH] don't export LINES and COLUMNS env vars in term to fix ncurses applications

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

> As for "causing more problems than it solves", I don't think I saw
> any evidence for that: even the one use case which started this bug
> report was later shown to behave as expected.

The recipe in the OP was:

> emacs -Q
> M-x term
> <ret> (to select /bin/bash)
> htop (to run htop)
> 
> and resizing the window (especially making it smaller) can make the
> program impossible to read.

When I do this on my machine, the output gets all messed up and
impossible to read.

Best regards,
Stefan Kangas

bug#37564: [PATCH] don't export LINES and COLUMNS env vars in term to fix ncurses applications

[Matthew Leach]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Matthew Leach <matthew@mattleach.net>
>> Cc: 37564@debbugs.gnu.org
>> Date: Tue, 01 Oct 2019 20:24:14 +0100
>> 
>> > I simply tried "env | fgrep LINES" and I do see the variable, although
>> > it wasn't there before I launched Emacs.
>> 
>> I would expect the variable to be there when inside Emacs as it has
>> exported it.  You should find that it won't exist if ran outside Emacs,
>> for example in xterm.
>
> Sorry, I'm confused.  Are we talking about running "M-x term" inside
> Emacs, or are we talking about something else?  The subject of the bug
> report says "term", so I assumed you mean term-mode.

Indeed, I'm talking about term-mode (M-x term). Sorry if I didn't make
that clear.

>> Ah, indeed if I try and resize and print the variable it has updated:
>> 
>> matthew@hopton ~ $ env | grep -i lines
>> LINES=30
>> matthew@hopton ~ $ env | grep -i lines
>> LINES=12
>> 
>> I think the issue is that the environment can't be while a process is
>> running; the environment variables are fixed when the program has
>> started. For a ncurses application this presents a problem as LINES and
>> COLUMNS can't be updated when the window size is changed and the program
>> handles a SIGWINCH. You can see that on the output above.
>
> While the process is running, Emacs sends ioctl commands to
> communicate the window size changes.

Sure but by exporting LINES and COLUMNS (something that other terminal
emulators don't do) programs aren't picking up changes in the window
size.

-- 
Matt

bug#37564: [PATCH] don't export LINES and COLUMNS env vars in term to fix ncurses applications

[Lars Ingebrigtsen]

Matthew Leach <matthew@mattleach.net> writes:

>> I simply tried "env | fgrep LINES" and I do see the variable, although
>> it wasn't there before I launched Emacs.
>
> I would expect the variable to be there when inside Emacs as it has
> exported it.  You should find that it won't exist if ran outside Emacs,
> for example in xterm.

I tried this in my bash here (under the "Terminal" program in Debian):

larsi@marnie:~/src/emacs/trunk$ set | grep LINES
LINES=24
larsi@marnie:~/src/emacs/trunk$ set | grep COLUMNS
COLUMNS=80

So they're in my env at least.

But I see I have this in my very ancient ~/.bashrc:

# check the window size after each command and, if necessary,
# update the values of LINES and COLUMNS.
shopt -s checkwinsize


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

bug#37564: [PATCH] don't export LINES and COLUMNS env vars in term to fix ncurses applications

[Andreas Schwab]

On Okt 07 2019, Lars Ingebrigtsen <larsi@gnus.org> wrote:

> I tried this in my bash here (under the "Terminal" program in Debian):
>
> larsi@marnie:~/src/emacs/trunk$ set | grep LINES
> LINES=24
> larsi@marnie:~/src/emacs/trunk$ set | grep COLUMNS
> COLUMNS=80
>
> So they're in my env at least.

`set' doesn't print the environment, it prints the shell variables.  To
print the environment, use `export -p' or `printenv'.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

bug#37564: [PATCH] don't export LINES and COLUMNS env vars in term to fix ncurses applications

[Lars Ingebrigtsen]

Andreas Schwab <schwab@linux-m68k.org> writes:

> `set' doesn't print the environment, it prints the shell variables.  To
> print the environment, use `export -p' or `printenv'.

I see.  Yes, they are not in my environment when I check with printenv.

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