svn commit: r314685 - head/bin/ps

[Conrad Meyer]

Hi Ed,

On Sat, Mar 4, 2017 at 2:58 PM, Ed Schouten <ed at nuxi.nl> wrote:
>> @@ -194,6 +194,8 @@ main(int argc, char *argv[])
>>
>>         if ((cols = getenv("COLUMNS")) != NULL && *cols != '\0')
>>                 termwidth = atoi(cols);
>> +       else if (!isatty(STDOUT_FILENO))
>> +               termwidth = UNLIMITED;
>>         else if ((ioctl(STDOUT_FILENO, TIOCGWINSZ, (char *)&ws) == -1 &&
>>              ioctl(STDERR_FILENO, TIOCGWINSZ, (char *)&ws) == -1 &&
>>              ioctl(STDIN_FILENO,  TIOCGWINSZ, (char *)&ws) == -1) ||
>>
>
> I think you can actually go ahead and simplify this a bit:
>
> - If something is a TTY, then our implementation of the TTY layer
> guarantees that TIOCGWINSZ always works.

Do you know if it did in 1990 too?  It's hard to tell why Marc@ made
this change way back then.  I wasn't sure so I left it alone.

> - If we're only interested in testing stdout whether it's a TTY, I
> think it makes little sense to check TIOCGWINSZ on stdin, stderr.
>
> I think there would therefore be very little harm to use something like this:
>
> |         if ((cols = getenv("COLUMNS")) != NULL && *cols != '\0')
> |                 termwidth = atoi(cols);
> |         else if ((ioctl(STDOUT_FILENO, TIOCGWINSZ, (char *)&ws) ==
> -1 && ws.ws_row == 0)

Shouldn't this remain || ws.ws_row == 0?

> |                 termwidth = UNLIMITED;
> |        else
> |                termwidth = ws.ws_col - 1;

I had a very similar cleanup in mind (|| instead of &&).  But I wasn't
sure if there was any reason TIOCGWINSZ might fail on stdout (and why
that change was added originally).

Best,
Conrad