netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ss: use compact output for undetected screen width
@ 2019-12-23 12:47 Peter Junos
  2019-12-25 20:36 ` Stephen Hemminger
  0 siblings, 1 reply; 6+ messages in thread
From: Peter Junos @ 2019-12-23 12:47 UTC (permalink / raw)
  To: netdev; +Cc: petoju

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(-)

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;
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] ss: use compact output for undetected screen width
  2019-12-23 12:47 [PATCH] ss: use compact output for undetected screen width Peter Junos
@ 2019-12-25 20:36 ` Stephen Hemminger
  2019-12-26 13:06   ` Peter Junos
  2019-12-26 13:07   ` [PATCH net-next] " Peter Junos
  0 siblings, 2 replies; 6+ messages in thread
From: Stephen Hemminger @ 2019-12-25 20:36 UTC (permalink / raw)
  To: Peter Junos; +Cc: netdev

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;
	}

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ss: use compact output for undetected screen width
  2019-12-25 20:36 ` Stephen Hemminger
@ 2019-12-26 13:06   ` Peter Junos
  2019-12-26 13:07   ` [PATCH net-next] " Peter Junos
  1 sibling, 0 replies; 6+ messages in thread
From: Peter Junos @ 2019-12-26 13:06 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev

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!

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH net-next] ss: use compact output for undetected screen width
  2019-12-25 20:36 ` Stephen Hemminger
  2019-12-26 13:06   ` Peter Junos
@ 2019-12-26 13:07   ` Peter Junos
  2020-01-02 18:42     ` David Ahern
  1 sibling, 1 reply; 6+ messages in thread
From: Peter Junos @ 2019-12-26 13:07 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Peter Junos, netdev

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. No width is assumed here.

To reproduce the issue, call
ss | less
and see every other line empty if your screen is between 80 and 160
columns wide.

This second version of the patch fixes screen_width being set to arbitrary
value.

Signed-off-by: Peter Junos <petoju@gmail.com>
---
 misc/ss.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index 95f1d37a..d5bae16d 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,9 +1159,15 @@ static int render_screen_width(void)
  */
 static void render_calc_width(void)
 {
-	int screen_width = render_screen_width();
+	int screen_width, first, len = 0, linecols = 0;
+	bool compact_output = false;
 	struct column *c, *eol = columns - 1;
-	int first, len = 0, linecols = 0;
+
+	screen_width = render_screen_width();
+	if (screen_width == -1) {
+		screen_width = INT_MAX;
+		compact_output = true;
+	}
 
 	/* First pass: set width for each column to measured content length */
 	for (first = 1, c = columns; c - columns < COL_MAX; c++) {
@@ -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;
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next] ss: use compact output for undetected screen width
  2019-12-26 13:07   ` [PATCH net-next] " Peter Junos
@ 2020-01-02 18:42     ` David Ahern
  0 siblings, 0 replies; 6+ messages in thread
From: David Ahern @ 2020-01-02 18:42 UTC (permalink / raw)
  To: Peter Junos, Stephen Hemminger; +Cc: netdev

On 12/26/19 6:07 AM, Peter Junos 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. No width is assumed here.
> 
> To reproduce the issue, call
> ss | less
> and see every other line empty if your screen is between 80 and 160
> columns wide.
> 
> This second version of the patch fixes screen_width being set to arbitrary
> value.
> 
> Signed-off-by: Peter Junos <petoju@gmail.com>
> ---
>  misc/ss.c | 19 +++++++++++++++----
>  1 file changed, 15 insertions(+), 4 deletions(-)
> 

...

> @@ -1159,9 +1159,15 @@ static int render_screen_width(void)
>   */
>  static void render_calc_width(void)
>  {
> -	int screen_width = render_screen_width();
> +	int screen_width, first, len = 0, linecols = 0;
> +	bool compact_output = false;
>  	struct column *c, *eol = columns - 1;
> -	int first, len = 0, linecols = 0;

moved the new bool after struct column to maintain reverse xmas tree and
applied to iproute2-next. Thanks,


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] ss: use compact output for undetected screen width
@ 2019-12-23 10:45 Peter Junos
  0 siblings, 0 replies; 6+ messages in thread
From: Peter Junos @ 2019-12-23 10:45 UTC (permalink / raw)
  To: netdev; +Cc: petoju

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.
---
 misc/ss.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index 95f1d37a..56d44f6b 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) {
+		/* Terminal width couldn't be guessed, don't extend the output */
+		return;
+	}
+
 	/* Second pass: find out newlines and distribute available spacing */
 	for (c = columns; c - columns < COL_MAX; c++) {
 		int pad, spacing, rem, last;
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-01-02 18:42 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-23 12:47 [PATCH] ss: use compact output for undetected screen width Peter Junos
2019-12-25 20:36 ` Stephen Hemminger
2019-12-26 13:06   ` Peter Junos
2019-12-26 13:07   ` [PATCH net-next] " Peter Junos
2020-01-02 18:42     ` David Ahern
  -- strict thread matches above, loose matches on Subject: below --
2019-12-23 10:45 [PATCH] " Peter Junos

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).