diffstat does not honor COLUMNS environment variable

diffstat:

 debian/patches/00list | 1
 debian/patches/01-check-columns-env-var.dpatch | 27 +++++++++++++++++
 diffstat-1.45/debian/changelog | 10 ++++++
 diffstat-1.45/debian/control | 5 +--
 diffstat-1.45/debian/rules | 9 ++++-
 5 files changed, 48 insertions(+), 4 deletions(-)

confirming prior to subscribing sponsorship team

Steve: can you take a look at it?

Michael: can you also forward the patch upstream?

On Mon, 2008-03-31 at 06:25 +0000, Daniel Holbach wrote:
> Michael: can you also forward the patch upstream?

Sorry, I forgot to mention that I had done that, too. I have emailed
the bare patch to the author of the diffstat program. I did both Ubuntu
and the upstream author at the same time. I have not heard back from
the upstream author yet, however.

 --- Mike

--
Michael B. Trausch <email address hidden>
home: 404-592-5746, 1 www.trausch.us
cell: 678-522-7934 im: <email address hidden>, jabber
Ubuntu Unofficial Backports Project: http://backports.trausch.us/

Hi Michael,

While it may seem obvious that we would want diffstat to use the $COLUMNS value from the environment, this change is a behavior change for anyone running diffstat in a terminal window that's not 80 columns wide, which may have unexpected side effects for tools that invoke diffstat. So I think it's better to defer this change until after the hardy release.

In the meantime, perhaps you could consider getting this change accepted into the Debian package, so that it will flow into intrepid automatically once the archive unfreezes? Also, you may want to consider whether the decision to honor the $COLUMNS value should be conditional on whether stdout points at a terminal - this is something that packages like, e.g., dpkg do, to good effect.

On Mon, 2008-03-31 at 20:04 +0000, Steve Langasek wrote:
> While it may seem obvious that we would want diffstat to use the
> $COLUMNS value from the environment, this change is a behavior change
> for anyone running diffstat in a terminal window that's not 80 columns
> wide, which may have unexpected side effects for tools that invoke
> diffstat. So I think it's better to defer this change until after the
> hardy release.
>
> In the meantime, perhaps you could consider getting this change
> accepted into the Debian package, so that it will flow into intrepid
> automatically once the archive unfreezes? Also, you may want to
> consider whether the decision to honor the $COLUMNS value should be
> conditional on whether stdout points at a terminal - this is something
> that packages like, e.g., dpkg do, to good effect.

I have submitted it upstream for consideration, and I can look into
getting it into Debian. Not sure what that would entail precisely, but
I can look at it and see what I can do with it.

Also you brought up a great point there about considering what stdout
is. I will add that to my patch shortly. Thanks!

--
Michael B. Trausch <email address hidden>
home: 404-592-5746, 1 www.trausch.us
cell: 678-522-7934 im: <email address hidden>, jabber
Ubuntu Unofficial Backports Project: http://backports.trausch.us/

Diffstat on attached patch, which now considers stdout's status as a tty before acting on COLUMNS:

 debian/patches/00list | 1
 debian/patches/01-check-columns-env-var.dpatch | 30 +++++++++++++++++++++++++
 diffstat-1.45/debian/changelog | 12 ++++++++++
 diffstat-1.45/debian/control | 5 ++--
 diffstat-1.45/debian/rules | 9 +++++--
 5 files changed, 53 insertions(+), 4 deletions(-)

Thinking about it more, though, I think I like dpkg's behavior, wherein COLUMNS is honored even when stdout is a process instead of a tty. For example if you run:

  COLUMNS=70 dpkg -l ubuntu-desktop | less

The output is properly formatted, whereas if you remove the COLUMNS=, you'll find that it spans nearly 140 columns. Either way, though, it works.

diffstat already has a -w option which can be used to achieve the same result.
Making it respond to the environment variable would change existing uses.

man diffstat

       -w number
              specify the maximum width of the histogram. The histogram will
              never be shorter than 10 columns, just in case the filenames get
              too large.

The request was for diffstat to honor the existing $COLUMNS env variable. Commandline options are orthogonal to this.

On Wed, 20 Jan 2010, Steve Langasek wrote:

> The request was for diffstat to honor the existing $COLUMNS env
> variable. Commandline options are orthogonal to this.

If you have some advice on how to not impact existing usage, you should
comment on that in the bug report.

--
Thomas E. Dickey
http://invisible-island.net
ftp://invisible-island.net

I've just applied the second patch in the 1.51-2 package I uploaded to unstable.

For whatever reason, this change will not appear in upstream,
since it adversely affects existing applications.

This bug was fixed in the package diffstat - 1.52-1

---------------
diffstat (1.52-1) unstable; urgency=low

  * New upstream release:
    - new "-C" option to show the histogram using SGR colors
    - new "-s" option to show only the summary line
  * Drop manpage hyphens patch (applied upstream)

diffstat (1.51-2) unstable; urgency=low

  * Adopt this package (closes: #588876)
  * Consider the COLUMNS environment variable when determining the default
    output width (LP: #209537). Thanks to Michael B. Trausch!
  * Add a patch to fix manpage hyphens (lintian notice)

  * Add homepage field to debian/control
  * Bump Standards Version to 3.9.0 and debhelper version to 7
  * Mention git repo in debian/control
  * Add a watch file for the ftp site
  * Switch to 3.0 (quilt) source package format
  * Use a minimal debian/rules makefile

diffstat (1.51-1) unstable; urgency=low

  * New upstream release.
    - Automagic detection and handling of compressed diffs. (closes: 491575)
    - Handle special case of no-newline message from some diffs. (closes: 563875)
  * Orphan this package (set maintainer to Debian QA Group).
 -- Alessio Treglia <email address hidden> Sat, 17 Jul 2010 10:35:59 +1200