Set debug output width in REPL

Set debug output width in REPL

[Mike Gran]

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

Hi-

I find that the backtrace output in the REPL is too constrained
my verbose code.  The attached patch would let one set the 

width of the backtrace and locals meta-commands.

What do you think?

-Mike

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-width-meta-command-to-set-screen-width-in-debug-.patch --]
[-- Type: text/x-patch; name="0001-Add-width-meta-command-to-set-screen-width-in-debug-.patch", Size: 3123 bytes --]

From 781bcc2783709e76591cd354f4976f02eb284526 Mon Sep 17 00:00:00 2001
From: Michael Gran <spk121@yahoo.com>
Date: Sun, 20 Feb 2011 21:53:46 -0800
Subject: [PATCH] Add ,width meta-command to set screen width in debug output

This meta-command allows one to set the default number of columns
that output from ,backtrace and ,locals shall occupy.

* doc/ref/scheme-using.texi (Debug Commands): document ,width
* module/system/repl/command.scm (*width*): new var
  (backtrace, locals): use *width* in optarg
  (width): new meta-command
---
 doc/ref/scheme-using.texi      |    6 ++++++
 module/system/repl/command.scm |   21 ++++++++++++++++++---
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/doc/ref/scheme-using.texi b/doc/ref/scheme-using.texi
index 126b845..a119d42 100644
--- a/doc/ref/scheme-using.texi
+++ b/doc/ref/scheme-using.texi
@@ -337,6 +337,12 @@ Show the VM registers associated with the current frame.
 @xref{Stack Layout}, for more information on VM stack frames.
 @end deffn
 
+@deffn {REPL Command} width [cols]
+Sets the number of display columns in the output of @code{,backtrace}
+and @code{,locals} to @var{cols}.  If @var{cols} is not given, the width
+of the terminal is used.
+@end deffn
+
 The next 3 commands work at any REPL.
 
 @deffn {REPL Command} break proc
diff --git a/module/system/repl/command.scm b/module/system/repl/command.scm
index d4b3e4a..40d720d 100644
--- a/module/system/repl/command.scm
+++ b/module/system/repl/command.scm
@@ -71,6 +71,8 @@
 (define *show-table*
   '((show (warranty w) (copying c) (version v))))
 
+(define *width* 72)
+
 (define (group-name g) (car g))
 (define (group-commands g) (cdr g))
 
@@ -546,7 +548,7 @@ Trace execution."
                  (format #t "Nothing to debug.~%"))))))))
 
 (define-stack-command (backtrace repl #:optional count
-                                 #:key (width 72) full?)
+                                 #:key (width *width*) full?)
   "backtrace [COUNT] [#:width W] [#:full? F]
 Print a backtrace.
 
@@ -626,12 +628,12 @@ With an argument, select a frame by index, then show it."
 Print the procedure for the selected frame."
   (repl-print repl (frame-procedure cur)))
 
-(define-stack-command (locals repl)
+(define-stack-command (locals repl #:key (width *width*))
   "locals
 Show local variables.
 
 Show locally-bound variables in the selected frame."
-  (print-locals cur))
+  (print-locals cur #:width width))
 
 (define-stack-command (error-message repl)
   "error-message
@@ -811,6 +813,19 @@ Print registers.
 Print the registers of the current frame."
   (print-registers cur))
 
+(define-meta-command (width repl #:optional x)
+  "width [X]
+Set debug output width.
+
+Set the number of screen columns in the output from `backtrace' and
+`locals'."
+  (if (and x (not (integer? x)))
+      (error "expected a column number (a non-negative integer)" x)
+      (let ((w (or x
+                   (false-if-exception (string->number (getenv "COLUMNS")))
+                   72)))
+        (format #t "Setting screen width to ~a columns~%" w)
+        (set! *width* w))))
 
 \f
 ;;;
-- 
1.7.4

Re: Set debug output width in REPL

[Mark H Weaver]

Mike Gran <spk121@yahoo.com> writes:
> I find that the backtrace output in the REPL is too constrained
> my verbose code.  The attached patch would let one set the 
> width of the backtrace and locals meta-commands.

> index d4b3e4a..40d720d 100644
> --- a/module/system/repl/command.scm
> +++ b/module/system/repl/command.scm
> @@ -71,6 +71,8 @@
>  (define *show-table*
>    '((show (warranty w) (copying c) (version v))))
>  
> +(define *width* 72)
> +
>  (define (group-name g) (car g))
>  (define (group-commands g) (cdr g))

> What do you think?

It seems to me that *width* should not be a global variable, but rather
a per-repl setting.  It probably belongs in the options field of the
<repl> record, no?  See "repl-default-options" in repl/common.scm.

    Thanks,
      Mark

Re: Set debug output width in REPL

[Mark H Weaver]

I wrote:
> It seems to me that *width* should not be a global variable, but rather
> a per-repl setting.  It probably belongs in the options field of the
> <repl> record, no?  See "repl-default-options" in repl/common.scm.

Better yet, maybe it should be an optional attribute of the output port,
which would allow pretty-print and truncated-print to use it as well.
Output ports attached to terminals could determine the terminal width
from the OS, on systems that support it.

What do you think?

    Mark

Re: Set debug output width in REPL

[Mike Gran]

> From:Mark H Weaver <mhw@netris.org>

> 
> Mark wrote:
> > It seems to me that *width* should not be a global variable, but rather
> > a per-repl setting.  It probably belongs in the options field of the
> > <repl> record, no?  See "repl-default-options" in 
> repl/common.scm.

For my personal case, when I run the REPL, I run it in a single terminal.
When new REPLs are spawned, they are recursive.  If we implemented such
a patch, it might make sense to have children inherit the width of their
parents.

How common is the use case where REPLs for a single Guile session appear
in terminals of different width?

> 
> Mark also wrote:
> Better yet, maybe it should be an optional attribute of the output port,
> which would allow pretty-print and truncated-print to use it as well.
> Output ports attached to terminals could determine the terminal width
> from the OS, on systems that support it.

This would also work, but, the width of terminals can change when windows
are resized.  So, updating that attribute would either have to be a manual
call to query the width and update it or it'd have to try to automatically
query the terminal for the width.  If one wanted to have a port automatically
query its terminal for the width, one could either (IIRC) catch the SIGWINCH
signalor could call a getenv/tget function before printing a pretty-print or
truncated-print.

> 
> What do you think?

The important thing for me is to be able to spread backtrace
information over a couple of terminal lines, so I'd like to be able to set
a default width that is greater than the terminal width.

Thanks,

Mike

Re: Set debug output width in REPL

[Ludovic Courtès]

Hi,

Mike Gran <spk121@yahoo.com> writes:

>> From:Mark H Weaver <mhw@netris.org>
>
>> 
>> Mark wrote:
>> > It seems to me that *width* should not be a global variable, but rather
>> > a per-repl setting.  It probably belongs in the options field of the
>> > <repl> record, no?  See "repl-default-options" in 
>> repl/common.scm.

[...]

> How common is the use case where REPLs for a single Guile session appear
> in terminals of different width?

Think of ‘--listen’.

>> Mark also wrote:
>> Better yet, maybe it should be an optional attribute of the output port,
>> which would allow pretty-print and truncated-print to use it as well.
>> Output ports attached to terminals could determine the terminal width
>> from the OS, on systems that support it.

I like the idea of using the terminal’s width, but I dislike the idea of
having it a property of the port, since it really seems orthogonal to me.

> [...] If one wanted to have a port automatically
> query its terminal for the width, one could either (IIRC) catch the SIGWINCH
> signalor could call a getenv/tget function before printing a pretty-print or
> truncated-print.

How about having a per-REPL setting that’s automatically set to the
terminal’s width by default?

What’s the exact call to get the width?

Thanks,
Ludo’.

Re: Set debug output width in REPL

[Mike Gran]

> From:Ludovic Courtès <ludo@gnu.org>
> How about having a per-REPL setting that’s automatically set to the
> terminal’s width by default?
> 
> What’s the exact call to get the width?

The lightweight way is to just check for the COLUMNS environment variable.
Most of the xterm-like terminals set COLUMNS when a program is executed.
That will cover most situations when running in X.

Beyond that, you'd need to look at the TERM variable and guess columns
from that.  For this you'd usually query the terminfo database, which means
that you probably end up with curses as a dependency.  It usually already
is, since readline depends on it.

Below please find a program that uses curses to find the terminal's
width.  It needs to be linked -lcurses.

Alternately you could execute "tput cols".

Thanks,

-Mike

#include <stdio.h>
#include <curses.h>
#include <term.h>
#define STDOUT_FILENO 1
#define DEFAULT_COLUMNS (80)
int main (int argc, char ** argv)
{
  int ret, err, cols;
  setupterm ((char *) 0, STDOUT_FILENO, &err);
  if (ret == OK || err == 1)
    {
      cols = tigetnum ("cols");
      if (cols < 0)
        cols = DEFAULT_COLUMNS;
    }
  else
    cols = DEFAULT_COLUMNS;
  printf("columns %d\n", cols);
  return 0;
}

Re: Set debug output width in REPL

[Mark H Weaver]

ludo@gnu.org (Ludovic Courtès) writes:
>> [...] If one wanted to have a port automatically
>> query its terminal for the width, one could either (IIRC) catch the SIGWINCH
>> signalor could call a getenv/tget function before printing a pretty-print or
>> truncated-print.
>
> How about having a per-REPL setting that’s automatically set to the
> terminal’s width by default?
>
> What’s the exact call to get the width?

There's an ioctl (TIOCGWINSZ) that queries the kernel's idea of the
terminal size.  See the autoconf macro AC_HEADER_TIOCGWINSZ and the
gnulib modules winsz-ioctl and winsz-termios.  Search for TIOCGWINSZ and
SIGWINCH in various packages (e.g. openssh and ncurses) to see how they
do it.  This is the rough idea:

AC_CHECK_HEADERS([termios.h])
AC_HEADER_TIOCGWINSZ


#if HAVE_TERMIOS_H
# include <termios.h>
#endif

#if GWINSZ_IN_SYS_IOCTL
# include <sys/ioctl.h>
#endif

#ifdef TIOCGWINSZ
  struct winsize ws;
  if (ioctl(term_file_descriptor, TIOCGWINSZ, &ws) < 0)
    ERROR;
  terminal_width = ws.ws_col;
#endif

Ideally this code would be called not only during initialization, but
also in response to the SIGWINCH signal.

    Best,
     Mark

Re: Set debug output width in REPL

[Andy Wingo]

Hi Mike,

On Mon 21 Feb 2011 07:02, Mike Gran <spk121@yahoo.com> writes:

> I find that the backtrace output in the REPL is too constrained
> my verbose code.  The attached patch would let one set the 
>
> width of the backtrace and locals meta-commands.
>
> What do you think?

Applied, thanks.  I hemmed and hawed about repl options and such but in
the end this is the simplest thing we can do.  I also added a patch to
default to the terminal width, without the need to ,width.

Regards,

Andy
-- 
http://wingolog.org/

Re: Set debug output width in REPL

[Ludovic Courtès]

Hi,

Andy Wingo <wingo@pobox.com> writes:

> On Mon 21 Feb 2011 07:02, Mike Gran <spk121@yahoo.com> writes:
>
>> I find that the backtrace output in the REPL is too constrained
>> my verbose code.  The attached patch would let one set the 
>>
>> width of the backtrace and locals meta-commands.
>>
>> What do you think?
>
> Applied, thanks.

FWIW I still think that the default should take into account terminal
settings, as discussed in this thread.

Thanks,
Ludo’.

Re: Set debug output width in REPL

[Andy Wingo]

On Sat 05 Mar 2011 14:26, ludo@gnu.org (Ludovic Courtès) writes:

> Andy Wingo <wingo@pobox.com> writes:
>
>> On Mon 21 Feb 2011 07:02, Mike Gran <spk121@yahoo.com> writes:
>>
>>> I find that the backtrace output in the REPL is too constrained
>>> my verbose code.  The attached patch would let one set the 
>>>
>>> width of the backtrace and locals meta-commands.
>>>
>>> What do you think?
>>
>> Applied, thanks.
>
> FWIW I still think that the default should take into account terminal
> settings, as discussed in this thread.

Sorry for the confusion.  The terminal settings are associated with an
fd, which maps more or less to a port, and I thought that you were
opposed to port-specific properties.  (Incidentally I did not think that
argument made much sense.)

Having it be a repl-specific property has the irritation of propagating
the width down to nested repls.  To me the true place of this setting
is specific to a thread, as repls are (largely) specific to threads.

So it would be equivalent to have `terminal-width' close over a fluid
instead of a lexical.

Anyway, with a week gone by and no more patches from anyone involved, I
decided to think about it myself for a bit, and ended up thinking Mike's
patch was "worse is better", but then also feeling it could be cleaner,
hence my addition.

Also, I wanted the initial settings to reflect the terminal width
without a need to call ,width.

So them's the reasons!  Let me know if you have any more concerns.  Do
try out different approaches -- it is a clarifying agent :)

Regards,

Andy
-- 
http://wingolog.org/

Re: Set debug output width in REPL

[Mark H Weaver]

ludo@gnu.org (Ludovic Courtès) writes:
>>> Mark also wrote:
>>> Better yet, maybe it should be an optional attribute of the output port,
>>> which would allow pretty-print and truncated-print to use it as well.
>>> Output ports attached to terminals could determine the terminal width
>>> from the OS, on systems that support it.
>
> I like the idea of using the terminal’s width, but I dislike the idea of
> having it a property of the port, since it really seems orthogonal to
> me.

I agree that it's not a perfect solution, but can you propose a better
one that might reasonably be adopted in practice?

Unfortunately, if we punt on this for want of perfection, we're likely
to end up with a nearly pessimal solution: each individual module that
needs to format output for fixed-width character displays will end up
with its own half-baked solution, such as the one just applied.

Ports are already very far from the simple and elegant ideas that
comprise most of Scheme, and they already include much functionality
that should be orthogonal.  Which is worse?  To add one more optional
attribute to ports, or to have several inconsistent and inflexible
notions of terminal width floating around the rest of the system?

     Mark