Debian Bug report logs - #697696
diffstat: Doesn't fetch terminal width correctly

version graph

Package: diffstat; Maintainer for diffstat is Sandro Tosi <morph@debian.org>; Source for diffstat is src:diffstat.

Reported by: Matthew Gabeler-Lee <cheetah@fastcat.org>

Date: Tue, 8 Jan 2013 16:03:02 UTC

Severity: normal

Found in version diffstat/1.55-3

Done: Sandro Tosi <morph@debian.org>

Bug is archived. No further changes may be made.

Toggle useless messages

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to debian-bugs-dist@lists.debian.org, Sandro Tosi <morph@debian.org>:
Bug#697696; Package diffstat. (Tue, 08 Jan 2013 16:03:05 GMT) Full text and rfc822 format available.

Acknowledgement sent to Matthew Gabeler-Lee <cheetah@fastcat.org>:
New Bug report received and forwarded. Copy sent to Sandro Tosi <morph@debian.org>. (Tue, 08 Jan 2013 16:03:06 GMT) Full text and rfc822 format available.

Message #5 received at submit@bugs.debian.org (full text, mbox):

From: Matthew Gabeler-Lee <cheetah@fastcat.org>
To: Debian Bug Tracking System <submit@bugs.debian.org>
Subject: diffstat: Doesn't fetch terminal width correctly
Date: Tue, 08 Jan 2013 10:59:27 -0500
Package: diffstat
Version: 1.55-3
Severity: normal

Diffstat tries to get the terminal width to auto-scale the histogram, but
the method it uses simply doesn't work, at least with bash.

The method it uses is getenv("COLUMNS").  Unfortunately, at least with bash,
the COLUMNS variable is normally a "magic" variable, it doesn't really exist
in the environment, and so diffstat always thinks the terminal is 80 chars
wide, which often results in forcing the histogram to only be its minimum 10
chars wide, no matter how big the terminal window is.

Workaround: export COLUMNS

-- System Information:
Debian Release: wheezy/sid
  APT prefers testing
  APT policy: (990, 'testing'), (500, 'unstable'), (1, 'experimental')
Architecture: amd64 (x86_64)
Foreign Architectures: i386

Kernel: Linux 3.6-trunk-amd64 (SMP w/8 CPU cores)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/bash

Versions of packages diffstat depends on:
ii  libc6  2.13-37

diffstat recommends no packages.

diffstat suggests no packages.

-- no debconf information



Information forwarded to debian-bugs-dist@lists.debian.org:
Bug#697696; Package diffstat. (Wed, 09 Jan 2013 22:54:03 GMT) Full text and rfc822 format available.

Acknowledgement sent to Sandro Tosi <morph@debian.org>:
Extra info received and forwarded to list. (Wed, 09 Jan 2013 22:54:03 GMT) Full text and rfc822 format available.

Message #10 received at 697696@bugs.debian.org (full text, mbox):

From: Sandro Tosi <morph@debian.org>
To: Matthew Gabeler-Lee <cheetah@fastcat.org>, 697696@bugs.debian.org
Subject: Re: Bug#697696: diffstat: Doesn't fetch terminal width correctly
Date: Wed, 9 Jan 2013 23:50:24 +0100
Hello Matthew,

On Tue, Jan 8, 2013 at 4:59 PM, Matthew Gabeler-Lee <cheetah@fastcat.org> wrote:
> Diffstat tries to get the terminal width to auto-scale the histogram, but
> the method it uses simply doesn't work, at least with bash.
>
> The method it uses is getenv("COLUMNS").  Unfortunately, at least with bash,
> the COLUMNS variable is normally a "magic" variable, it doesn't really exist
> in the environment, and so diffstat always thinks the terminal is 80 chars
> wide, which often results in forcing the histogram to only be its minimum 10
> chars wide, no matter how big the terminal window is.

By default, diffstat doesn't try to identify the terminal width at
all, but uses a default 80 chars columns width value. It was a recent
patch[1] that introduces the behaviour of checking COLUMNS (else you'd
have to use -w NUM). So the current situation might be better than the
static columns width in same cases, so can you elaborate a bit more
what you want from this bug report (I would close it actually)? :)

[1] http://patch-tracker.debian.org/patch/series/view/diffstat/1.55-3/01-check-columns-env-var.patch

Cheers,
--
Sandro Tosi (aka morph, morpheus, matrixhasu)
My website: http://matrixhasu.altervista.org/
Me at Debian: http://wiki.debian.org/SandroTosi



Reply sent to Sandro Tosi <morph@debian.org>:
You have taken responsibility. (Sun, 13 Jan 2013 11:39:06 GMT) Full text and rfc822 format available.

Notification sent to Matthew Gabeler-Lee <cheetah@fastcat.org>:
Bug acknowledged by developer. (Sun, 13 Jan 2013 11:39:06 GMT) Full text and rfc822 format available.

Message #15 received at 697696-done@bugs.debian.org (full text, mbox):

From: Sandro Tosi <morph@debian.org>
To: 697696-done@bugs.debian.org
Subject: Re: Bug#697696: diffstat: Doesn't fetch terminal width correctly
Date: Sun, 13 Jan 2013 12:33:56 +0100
On Wed, Jan 9, 2013 at 11:50 PM, Sandro Tosi <morph@debian.org> wrote:
> By default, diffstat doesn't try to identify the terminal width at
> all, but uses a default 80 chars columns width value. It was a recent
> patch[1] that introduces the behaviour of checking COLUMNS (else you'd
> have to use -w NUM). So the current situation might be better than the
> static columns width in same cases, so can you elaborate a bit more
> what you want from this bug report (I would close it actually)? :)
>
> [1] http://patch-tracker.debian.org/patch/series/view/diffstat/1.55-3/01-check-columns-env-var.patch

Delivery to the following recipient failed permanently:

     cheetah@fastcat.org

Technical details of permanent failure:
Google tried to deliver your message, but it was rejected by the
recipient domain. We recommend contacting the other email provider for
further information about the cause of this error. The error that the
other server returned was: 451 451-209.85.212.171 is not yet
authorized to deliver mail from 451 <me> to <cheetah@fastcat.org>.
Please try later. (state 13).

I don't see a reason to keep this bug opened when I can't contact the
submitter, closing.

Regards,
--
Sandro Tosi (aka morph, morpheus, matrixhasu)
My website: http://matrixhasu.altervista.org/
Me at Debian: http://wiki.debian.org/SandroTosi



Information forwarded to debian-bugs-dist@lists.debian.org, Sandro Tosi <morph@debian.org>:
Bug#697696; Package diffstat. (Mon, 14 Jan 2013 16:51:03 GMT) Full text and rfc822 format available.

Acknowledgement sent to Matthew Gabeler-Lee <cheetah@fastcat.org>:
Extra info received and forwarded to list. Copy sent to Sandro Tosi <morph@debian.org>. (Mon, 14 Jan 2013 16:51:03 GMT) Full text and rfc822 format available.

Message #20 received at 697696@bugs.debian.org (full text, mbox):

From: Matthew Gabeler-Lee <cheetah@fastcat.org>
To: 697696@bugs.debian.org
Subject: Re: Bug#697696 closed by Sandro Tosi <morph@debian.org> (Re: Bug#697696: diffstat: Doesn't fetch terminal width correctly)
Date: Mon, 14 Jan 2013 11:20:29 -0500 (EST)
reopen 697696
thanks

On Sun, 13 Jan 2013, Debian Bug Tracking System wrote:

> Delivery to the following recipient failed permanently:
>
>      cheetah@fastcat.org
> 
> Technical details of permanent failure:
> Google tried to deliver your message, but it was rejected by the
> recipient domain. We recommend contacting the other email provider for
> further information about the cause of this error. The error that the
> other server returned was: 451 451-209.85.212.171 is not yet
> authorized to deliver mail from 451 <me> to <cheetah@fastcat.org>.
> Please try later. (state 13).
> 
> I don't see a reason to keep this bug opened when I can't contact the
> submitter, closing.

Apparently google has SMTP FAIL ... 451 is a temporary error, used in this
case for greylisting.

Anyways, my point in the bug report is that fetching the COLUMNS 
environment variable is a no-op, it is completely non-functional.

To get the terminal size, you need to do something e.g. with termcap or 
termios or such.

Alternately, one way to work around this without sucking in a dependency 
like that would be to rename /usr/bin/diffstat to diffstat.real, and 
have a trivial shell script like the following for /usr/bin/diffstat to 
make the COLUMNS variable visible to the real diffstat program:

#!/bin/bash
export COLUMNS
exec "`dirname "$0"`"/diffstat.real "$@"




Information forwarded to debian-bugs-dist@lists.debian.org:
Bug#697696; Package diffstat. (Thu, 24 Jan 2013 22:54:03 GMT) Full text and rfc822 format available.

Acknowledgement sent to Sandro Tosi <morph@debian.org>:
Extra info received and forwarded to list. (Thu, 24 Jan 2013 22:54:03 GMT) Full text and rfc822 format available.

Message #25 received at 697696@bugs.debian.org (full text, mbox):

From: Sandro Tosi <morph@debian.org>
To: Matthew Gabeler-Lee <cheetah@fastcat.org>, 697696@bugs.debian.org
Subject: Re: Bug#697696: closed by Sandro Tosi <morph@debian.org> (Re: Bug#697696: diffstat: Doesn't fetch terminal width correctly)
Date: Thu, 24 Jan 2013 23:50:45 +0100
On Mon, Jan 14, 2013 at 5:20 PM, Matthew Gabeler-Lee
<cheetah@fastcat.org> wrote:
> Apparently google has SMTP FAIL ... 451 is a temporary error, used in this
> case for greylisting.

likely... for 4 days? you're probably doing something wrong there, anyway...

> Anyways, my point in the bug report is that fetching the COLUMNS environment
> variable is a no-op, it is completely non-functional.

have you tried it?

$ diff -urNad pp-1.6.2 pp-1.6.3 | diffstat
 CHANGELOG      |    7 ++
 PKG-INFO       |    4 -
 doc/ppdoc.html |  166 +++++++++++++++++++++++++++++++++------------------------
 doc/ppserver.1 |    3 +
 pp.py          |    3 -
 ppauto.py      |    2
 ppcommon.py    |    2
 ppserver.py    |   45 ++++++++++++---
 pptransport.py |    2
 ppworker.py    |    2
 10 files changed, 150 insertions(+), 86 deletions(-)

$ diff -urNad pp-1.6.2 pp-1.6.3 | COLUMNS=20 diffstat
 CHANGELOG      |    7
 PKG-INFO       |    4
 doc/ppdoc.html |  166 +++++-----
 doc/ppserver.1 |    3
 pp.py          |    3
 ppauto.py      |    2
 ppcommon.py    |    2
 ppserver.py    |   45 ++
 pptransport.py |    2
 ppworker.py    |    2
 10 files changed, 150 insertions(+), 86 deletions(-)

and of course it also works exporting COLUMNS and then running
diffstat, but that will change the global env instead of just the one
diffstat is executed (which might have other undesired effect).

So either you say what's wrong/doesn't work or else I don't see the
point of this report and I'll reclose it.

Regards,
--
Sandro Tosi (aka morph, morpheus, matrixhasu)
My website: http://matrixhasu.altervista.org/
Me at Debian: http://wiki.debian.org/SandroTosi



Information forwarded to debian-bugs-dist@lists.debian.org, Sandro Tosi <morph@debian.org>:
Bug#697696; Package diffstat. (Fri, 25 Jan 2013 14:24:03 GMT) Full text and rfc822 format available.

Acknowledgement sent to Matthew Gabeler-Lee <cheetah@fastcat.org>:
Extra info received and forwarded to list. Copy sent to Sandro Tosi <morph@debian.org>. (Fri, 25 Jan 2013 14:24:03 GMT) Full text and rfc822 format available.

Message #30 received at 697696@bugs.debian.org (full text, mbox):

From: Matthew Gabeler-Lee <cheetah@fastcat.org>
To: Sandro Tosi <morph@debian.org>,697696@bugs.debian.org
Subject: Re: Bug#697696: closed by Sandro Tosi <morph@debian.org> (Re: Bug#697696: diffstat: Doesn't fetch terminal width correctly)
Date: Fri, 25 Jan 2013 09:21:24 -0500
[Message part 1 (text/plain, inline)]

Sandro Tosi <morph@debian.org> wrote:

>On Mon, Jan 14, 2013 at 5:20 PM, Matthew Gabeler-Lee
><cheetah@fastcat.org> wrote:
>> Anyways, my point in the bug report is that fetching the COLUMNS
>environment
>> variable is a no-op, it is completely non-functional.
>
>have you tried it?

Yes ... hence the bug report ...

>$ diff -urNad pp-1.6.2 pp-1.6.3 | diffstat

Try with a diff with long paths and an even wider window, e.g. paths > 80 chars and a terminal / window > 100 columns.  It will not use the full terminal width, but instead use its minimum histogram width of 10 chars.

>$ diff -urNad pp-1.6.2 pp-1.6.3 | COLUMNS=20 diffstat

Aah, but here you are explicitly setting the COLUMNS value.

My point was that, unless you do something to explicitly expose COLUMNS to diffstat, it isn't available.  In your first sample, if you ran it under gdb, you would find that the fetch of the COLUMNS environment var returned NULL.  You can demonstrate that simply by just running "env | grep COLUMNS".

COLUMNS is an automatic / magic variable, at least in bash, and is not exported to sub-processes by default, so fetching it from a sub-process is not a functional way to query the terminal size.  I don't know if other shells behave differently from bash in that regard.

My point to the report is that if diffstat wants to find out the terminal size, it should actually query the terminal, instead of hoping that the user would have told their shell to tell diffstat what the terminal size is.  If I have to manually send the COLUMNS variable to diffstat, it's no better than a command line option.  Exporting COLUMNS from the shell by default is a poor option, I think, because it can cause many programs you might launch from the shell to then not react to terminal size changes, as they will treat having it set as a request to override whatever the reported terminal size is.  I actually use that feature sometimes to make reading man pages easier in otherwise very large terminal windows.

If using the termcap type facilities is too "heavy weight" for diffstat, it could farm out to stty or some other program that does query the terminal size "properly".
[Message part 2 (text/html, inline)]

Bug archived. Request was from Debbugs Internal Request <owner@bugs.debian.org> to internal_control@bugs.debian.org. (Sat, 23 Feb 2013 07:29:18 GMT) Full text and rfc822 format available.

Send a report that this bug log contains spam.


Debian bug tracking system administrator <owner@bugs.debian.org>. Last modified: Sat Apr 19 00:33:50 2014; Machine Name: buxtehude.debian.org

Debian Bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.