| Message ID | 20191223124716.GA25816@peto-laptopnovy |
|---|---|
| State | Changes Requested |
| Delegated to: | stephen hemminger |
| Headers | show |
| Series | ss: use compact output for undetected screen width | expand |
On Mon, 23 Dec 2019 13:47:16 +0100 Peter Junos <petoju@gmail.com> wrote: > This change fixes calculation of width in case user pipes the output. > > SS output output works correctly when stdout is a terminal. When one > pipes the output, it tries to use 80 or 160 columns. That adds a > line-break if user has terminal width of 100 chars and output is of > the similar width. > > To reproduce the issue, call > ss | less > and see every other line empty if your screen is between 80 and 160 > columns wide. > > Signed-off-by: Peter Junos <petoju@gmail.com> I would prefer that if the use pipes the command output to a pipe that the line length was assumed to be infinite. > @@ -1159,7 +1159,13 @@ static int render_screen_width(void) > */ > static void render_calc_width(void) > { > + bool compact_output = false; > int screen_width = render_screen_width(); > + if (screen_width == -1) { > + screen_width = 80; > + compact_output = true; > + } > + > struct column *c, *eol = columns - 1; > int first, len = 0, linecols = 0; > With this patch, declarations and code are now mixed (more than before). I would expect something like: static void render_calc_width(void) { int screen_width, first, len = 0, linecols = 0; bool compact_output = false; struct column *c, *eol = columns - 1; screen_width = render_screen_width(); if (screen_width == -1) { screen_width = INT_MAX; compact_output = true; }
On Wed, Dec 25, 2019 at 12:36:07PM -0800, Stephen Hemminger wrote: > On Mon, 23 Dec 2019 13:47:16 +0100 > Peter Junos <petoju@gmail.com> wrote: > > > This change fixes calculation of width in case user pipes the output. > > > > SS output output works correctly when stdout is a terminal. When one > > pipes the output, it tries to use 80 or 160 columns. That adds a > > line-break if user has terminal width of 100 chars and output is of > > the similar width. > > > > To reproduce the issue, call > > ss | less > > and see every other line empty if your screen is between 80 and 160 > > columns wide. > > > > Signed-off-by: Peter Junos <petoju@gmail.com> > > I would prefer that if the use pipes the command output to a pipe that > the line length was assumed to be infinite. Good point, this is used in min(). > > @@ -1159,7 +1159,13 @@ static int render_screen_width(void) > > */ > > static void render_calc_width(void) > > { > > + bool compact_output = false; > > int screen_width = render_screen_width(); > > + if (screen_width == -1) { > > + screen_width = 80; > > + compact_output = true; > > + } > > + > > struct column *c, *eol = columns - 1; > > int first, len = 0, linecols = 0; > > > > With this patch, declarations and code are now mixed (more than before). > I would expect something like: > > static void render_calc_width(void) > { > int screen_width, first, len = 0, linecols = 0; > bool compact_output = false; > struct column *c, *eol = columns - 1; > > screen_width = render_screen_width(); > if (screen_width == -1) { > screen_width = INT_MAX; > compact_output = true; > } Seems good, I'll send changed patch soon. Thanks for review!
diff --git a/misc/ss.c b/misc/ss.c index 95f1d37a..dff1a618 100644 --- a/misc/ss.c +++ b/misc/ss.c @@ -1135,10 +1135,10 @@ static void buf_free_all(void) buffer.chunks = 0; } -/* Get current screen width, default to 80 columns if TIOCGWINSZ fails */ +/* Get current screen width, returns -1 if TIOCGWINSZ fails */ static int render_screen_width(void) { - int width = 80; + int width = -1; if (isatty(STDOUT_FILENO)) { struct winsize w; @@ -1159,7 +1159,13 @@ static int render_screen_width(void) */ static void render_calc_width(void) { + bool compact_output = false; int screen_width = render_screen_width(); + if (screen_width == -1) { + screen_width = 80; + compact_output = true; + } + struct column *c, *eol = columns - 1; int first, len = 0, linecols = 0; @@ -1183,6 +1189,11 @@ static void render_calc_width(void) first = 0; } + if (compact_output) { + /* Compact output, skip extending columns. */ + return; + } + /* Second pass: find out newlines and distribute available spacing */ for (c = columns; c - columns < COL_MAX; c++) { int pad, spacing, rem, last;
This change fixes calculation of width in case user pipes the output. SS output output works correctly when stdout is a terminal. When one pipes the output, it tries to use 80 or 160 columns. That adds a line-break if user has terminal width of 100 chars and output is of the similar width. To reproduce the issue, call ss | less and see every other line empty if your screen is between 80 and 160 columns wide. Signed-off-by: Peter Junos <petoju@gmail.com> --- misc/ss.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)