[Patch] Resizing shells in shell-mode

[Patch] Resizing shells in shell-mode

[Antoine Levitt]

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

Hi,
Shell-mode doesn't update the COLUMNS env var, which prevents from having
pretty ls for example when resizing the window. I made a patch for this, but
I'm not sure how it should be inserted into main code, so I'm submitting it
in the hope someone will do that. It should be straightforward though.
Here's the code, to be included in shell.el

;;listen for window configuration changes to modify COLUMNS
(add-hook 'window-configuration-change-hook
      (lambda ()
        (if (eq major-mode 'shell-mode) (change-columns))))

;;filters one and exactly one input
(defun filter-all (string)
  (remove-hook 'comint-preoutput-filter-functions 'filter-all)
  "")

;;tells the shell to change column, and discard reply (which should
;;be a prompt)
(defun change-columns ()
  (add-hook 'comint-preoutput-filter-functions 'filter-all)
  (funcall comint-input-sender
       (get-buffer-process (current-buffer))
       (format "export COLUMNS=%d" (window-width)))
  (accept-process-output))

[-- Attachment #2: Type: text/html, Size: 1243 bytes --]

Re: [Patch] Resizing shells in shell-mode

[Stefan Monnier]

> Shell-mode doesn't update the COLUMNS env var, which prevents from having
> pretty ls for example when resizing the window. I made a patch for this, but
> I'm not sure how it should be inserted into main code, so I'm submitting it
> in the hope someone will do that. It should be straightforward though.
> Here's the code, to be included in shell.el

> ;;listen for window configuration changes to modify COLUMNS
> (add-hook 'window-configuration-change-hook
>       (lambda ()
>         (if (eq major-mode 'shell-mode) (change-columns))))

> ;;filters one and exactly one input
> (defun filter-all (string)
>   (remove-hook 'comint-preoutput-filter-functions 'filter-all)
>   "")

> ;;tells the shell to change column, and discard reply (which should
> ;;be a prompt)
> (defun change-columns ()
>   (add-hook 'comint-preoutput-filter-functions 'filter-all)
>   (funcall comint-input-sender
>        (get-buffer-process (current-buffer))
>        (format "export COLUMNS=%d" (window-width)))
>   (accept-process-output))

Problem is: sending "export COLUMNS=%d" won't do the right thing if the
process running currently isn't some bourne derivative.  E.g. if it's
`csh' or some completely different process.


        Stefan

Re: [Patch] Resizing shells in shell-mode

[Antoine Levitt]

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

Oops, I messed up when replying, so the messages didn't go to the list. Here
they are :
Me :
Yup, that's a problem. I don't know about other shells so I just made it for
sh/bash. I don't think there is a way to change the environment of a process
without giving it some kind of command. Ideal would be to switch between
different commands according to the shell type, or fail without doing
anything if the shell is unknown. I'm a newbie to emacs and lisp, so I just
made the code using what I could find. Hopefully someone will turn this
snippet into something worthy to be merged into main emacs (even if it
doesn't work for every shell known to man)

Stefan Monnier :
The problem is not too serious w.r.t shell-compatibility (you can make
it configurable).  It's more serious when you take into account the fact
that some other process (not a shell) might be running at that moment.
Have you try M-x term instead of M-x shell?  It's a different tradeoff,
but it should be able to accomodate window-size changes.


Me:
Well, that's the reason why the patch is to shell, not comint. Isn't shell
the specialized mode for dealing with shells only ?

[-- Attachment #2: Type: text/html, Size: 1274 bytes --]

Re: [Patch] Resizing shells in shell-mode

[Stefan Monnier]

> Well, that's the reason why the patch is to shell, not comint. Isn't shell
> the specialized mode for dealing with shells only ?

I don't know about you, but when I use shell-mode, I don't just sit
there staring at the prompt.  ;-)
Instead I use it for what shells are for: to run other processes.

So even though you're in "shell-mode" and a shell is running under it,
the process that is "current" may be anything else than a shell.


        Stefan

Re: [Patch] Resizing shells in shell-mode

[Miles Bader]

"Antoine Levitt" <smeuuh@gmail.com> writes:
> It's more serious when you take into account the fact
> that some other process (not a shell) might be running at that moment.
> Have you try M-x term instead of M-x shell?  It's a different tradeoff,
> but it should be able to accomodate window-size changes.
>
> Me:
> Well, that's the reason why the patch is to shell, not comint. Isn't shell
> the specialized mode for dealing with shells only ?

Shells are often running other programs...

Sending shell commands without an explicit user action is a bad idea,
don't do it.

-Miles

-- 
`To alcohol!  The cause of, and solution to,
 all of life's problems' --Homer J. Simpson

Re: [Patch] Resizing shells in shell-mode

[Antoine Levitt]

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

And I did it again, sorry. Here is the original message :
Aww, I can't believe I haven't thought about this earlier. You're right, of
course. I'm sorry for not thinking this through
(this problem also occurs with shell-copy-environment-variable, but I don't
know how this is used so it may actually not be a problem)
Hmm, so, a solution would be to check for a prompt (there's already a regexp
to do just that) and only change columns when there's one on the line (so it
also matches prompts with a partially typed command) (bad, might be
noticeable under certain (rare) circumstances), or otherwise change the
value of $COLUMNS (better). I'm gonna check how gnome-terminal and such
things handle this.
Sorry again
Antoine


I think I have found the solution, which is to send a TIOCSWINSZ ioctl to
the tty. I'm not sure how to do that in elisp, and not sure how portable
that will be.
Antoine
2008/5/24 Miles Bader <miles@gnu.org>:

> "Antoine Levitt" <smeuuh@gmail.com> writes:
> > It's more serious when you take into account the fact
> > that some other process (not a shell) might be running at that moment.
> > Have you try M-x term instead of M-x shell?  It's a different tradeoff,
> > but it should be able to accomodate window-size changes.
> >
> > Me:
> > Well, that's the reason why the patch is to shell, not comint. Isn't
> shell
> > the specialized mode for dealing with shells only ?
>
> Shells are often running other programs...
>
> Sending shell commands without an explicit user action is a bad idea,
> don't do it.
>
> -Miles
>
> --
> `To alcohol!  The cause of, and solution to,
>  all of life's problems' --Homer J. Simpson
>

[-- Attachment #2: Type: text/html, Size: 2167 bytes --]

Re: [Patch] Resizing shells in shell-mode

[Jan Djärv]

Stefan Monnier skrev:

> Problem is: sending "export COLUMNS=%d" won't do the right thing if the
> process running currently isn't some bourne derivative.  E.g. if it's
> `csh' or some completely different process.

I think the right thing to do is to set the new size on the pty with ioctl 
TIOCSWINSZ and then send SIGWINCH to the inferior process.  AFAIK, shells 
react on that to set COLUMNS and LINES.  The signal is discarded by default so 
it does no harm if not caught.

	Jan D.

Re: [Patch] Resizing shells in shell-mode

[Antoine Levitt]

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

Hi,
I tested it, it works. I used a simple C program :
#include <sys/ioctl.h>
#include <stdio.h>
#include <fcntl.h>
int main(int argc, char **argv)
{
    int fd = open(argv[1], O_RDWR);
    struct winsize sz;
    sz.ws_row = atoi(argv[2]);
    sz.ws_col = atoi(argv[3]);
    ioctl(fd, TIOCSWINSZ, &sz);
    return 0;
}

Then in lisp, I added a hook to window-configuration-change-hook to call
this program with (process-tty-name), (window-width) and (window-height).
Interestingly, it worked in bash without needing a SIGWINCH, even when
deactivating the bash option in shopt that tells it to check for window size
changes.
I searched in the lisp manual, I didn't find any way to do what I did in C
in lisp. Is it possible ? Else a wrapper either to ioctl or directly
something like (change-size ttyname row col) (I don't know which one is
better) would need to be implemented in the C source code.
The resulting code in shell.el would like like :
(add-hook 'window-configuration-change-hook
      (lambda ()
        (if (eq major-mode 'shell-mode) (change-size))))

(defun change-size ()
  (ioctl (process-tty-name) (window-height) (window-size))
  (signal-process (get-buffer-process (current-buffer)) 'SIGWINCH))

There's many issues going on here I don't have any clue on how to solve, but
should be easy for an emacs developper : namespace cluttering, error
treatment (when a shell is stopped for example), performance, portability,
addition of the C source code ... could anyone tackle these in order to
solve the bug ?

Antoine
2008/5/25 Jan Djärv <jan.h.d@swipnet.se>:

>
>
> Stefan Monnier skrev:
>
>  Problem is: sending "export COLUMNS=%d" won't do the right thing if the
>> process running currently isn't some bourne derivative.  E.g. if it's
>> `csh' or some completely different process.
>>
>
> I think the right thing to do is to set the new size on the pty with ioctl
> TIOCSWINSZ and then send SIGWINCH to the inferior process.  AFAIK, shells
> react on that to set COLUMNS and LINES.  The signal is discarded by default
> so it does no harm if not caught.
>
>        Jan D.
>

[-- Attachment #2: Type: text/html, Size: 2891 bytes --]

Re: [Patch] Resizing shells in shell-mode

[Miles Bader]

"Antoine Levitt" <smeuuh@gmail.com> writes:
> Interestingly, it worked in bash without needing a SIGWINCH, even when
> deactivating the bash option in shopt that tells it to check for window size
> changes.

The SIGWINCH is sent automatically by the ioctl, so I don't think you
need to do it yourself.

-Miles

-- 
Idiot, n. A member of a large and powerful tribe whose influence in human
affairs has always been dominant and controlling.

Re: [Patch] Resizing shells in shell-mode

[Stefan Monnier]

> I tested it, it works. I used a simple C program :
> #include <sys/ioctl.h>
> #include <stdio.h>
> #include <fcntl.h>
> int main(int argc, char **argv)
> {
>     int fd = open(argv[1], O_RDWR);
>     struct winsize sz;
>     sz.ws_row = atoi(argv[2]);
>     sz.ws_col = atoi(argv[3]);
>     ioctl(fd, TIOCSWINSZ, &sz);
>     return 0;
> }

There's already a Lisp function to do it (written in C), called
`set-process-window-size'.


        Stefan