Bug 314195

Comment 1 Mirko Raner 2010-05-25 03:13:42 EDT

This can probably be fixed by creating a TextCanvas.ResizeListener that calls PTY.setTerminalSize(int, int) when a resize was detected.

A few minor obstacles are:

- addResizeHandler(ResizeListener) is a method of TextCanvas, which does not
  look like it's meant to be API, even for other terminal plug-ins; somehow
  this method needs to be lifted into API
- LocalTerminalLaunchDelegate does currently not store the PTY instance (and
  it cannot be retrieved from the Spawner); it depends on the particular
  choice of refactoring for addResizeHandler(...) how this will be best
  addressed

Comment 2 Martin Oberhuber 2010-05-25 23:45:41 EDT

I don't think we need any new API for relaying the terminal size - we already have ITerminalConnector#setTerminalSize() being called by the Framework as needed.

For SSH, this is implemented in
org.eclipse.tm.internal.terminal.ssh.SshConnector.setTerminalSize(int, int)
and it looks like LocalTerminalConnector simply needs to implement this.

Comment 3 Mirko Raner 2010-05-25 23:58:26 EDT

Thanks, Martin, I totally overlooked that method! That definitely makes things easier.

Comment 4 Mirko Raner 2010-05-26 02:08:12 EDT

Created attachment 169935 [details]
Initial patch (as .zip file) for passing terminal size changes to PTY

I'm having trouble connecting to dev.eclipse.org (206.191.52.50) right now, so I couldn't prepare a proper patch. The attached zip contains the files that I changed.

At least on my machine (Mac OS 10.5.8) this doesn't seem to fix the issue, though. When I type "stty -a|grep rows" I get

speed 9600 baud; 0 rows; 0 columns;

This doesn't change even after multiple resizing of the terminal view. However, after issuing a "resize" command the proper terminal size is returned by stty.
vi itself seems to have trouble picking up the changes as well, but I'm not sure whether this is a problem with vi, Mac OS, or the Local Terminal Connector.

Comment 5 Martin Oberhuber 2010-05-26 05:37:21 EDT

Created attachment 169959 [details]
patch v1

Attached is the fix as a patch (plus one copyright year update).

The patch does improve the situation, but it is incomplete: The remaining problem is that in my local terminal, the TERM variable is not set properly. On my linux host, TERM remains set as "xterm", which leads to sending escape sequences that the Terminal does not understand. TERM must be set as "ansi".

After manually doing a 
   setenv TERM ansi
vi follows resize-events as expected.

Mirko, can you please re-attach the patch as a patch such that I can add an iplog+ mark for proper IP reporting. I cannot officially accept your change as a ZIP.

Comment 6 Martin Oberhuber 2010-05-26 05:52:43 EDT

I believe that somehow the PTY should set the terminal name. At least on SSH and Telnet, this is functionality provided by the connection, such that the user doesn't have to set an environment variable.

Comment 7 Mirko Raner 2010-05-26 14:09:09 EDT

Created attachment 170063 [details]
Re-attached patch per Martin's request

Comment 8 Mirko Raner 2010-05-26 14:17:13 EDT

(In reply to comment #6)
> I believe that somehow the PTY should set the terminal name.

Hm, I'm not sure exactly how that it supposed to work. I don't see how I could communicate the terminal type to the pty driver.

> At least on SSH
> and Telnet, this is functionality provided by the connection, such that the
> user doesn't have to set an environment variable.

For now, what I can do is to automatically set the TERM variable in the environment settings of the launch configuration. Users can also modify the value manually on the "Environment" tab of the Terminal launch configuration dialog.

I'll provide a patch for this later today.

Comment 9 Martin Oberhuber 2010-05-26 14:18:57 EDT

Comment on attachment 169959 [details]
patch v1

sounds good to me, thanks

Comment 10 Mirko Raner 2010-05-27 02:44:04 EDT

Created attachment 170139 [details]
Patch including terminal resizing and TERM=ansi default setting

This patch includes the changes from the previous patch as well as the additional code to set TERM=ansi in the launch environment. Existing launch configurations need to be updated manually.

Also, as I was already working on the code anyway, the patch includes a fix for item 4 from bug 309899 comment #18 ("The name of the generated Launch ("Terminal (cmd.exe)") is not properly externalized").

Comment 11 Martin Oberhuber 2010-05-27 05:24:36 EDT

Comment on attachment 170139 [details]
Patch including terminal resizing and TERM=ansi default setting

Nice! - vi and resizing work as expected now.

In terms of the code, I only noticed that a simpler way than using MessageFormat is the NLS.bind() methods.

In terms of functionality, I noticed that my local terminal sometimes doesn't produce a prompt. I have filed bug 314625 for this.

Comment 12 Martin Oberhuber 2010-05-27 05:24:53 EDT

Released > I20100527.