* [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).