[pacman-dev] [PATCH] Fix list_display on non-ttys and other output fixes
commit c1f742d7750a14 broke what was one of the tenants of out output- if piping pacman output somewhere else, we shouldn't ever try to line-wrap and indent print our output. This makes it easier for tools to use output from pacman -Ss, -Qs, -Qi, etc. list_display() unfortunately was given a default value of 80 rather than 0, so fix this. Next, make some additional changes that ensure we don't insert an unnecessary blank line if for some crazy reason the indent level (such as on -Qi output) is greater than the number of columns. Accomplish this by printing the first item unconditionally as we do in list_display_linebreak(). Finally, teach indentprint to not wrap if the number of columns is less than the indent level, this prevents some forms of ridiculous output such as the following: Install Date : Wed 08 Jun 2011 04:39:19 AM CDT Signed-off-by: Dan McGee <dan@archlinux.org> --- For maint. Dave, looking forward to your feedback. :) -Dan src/pacman/util.c | 19 ++++++++++++------- 1 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/pacman/util.c b/src/pacman/util.c index 508cc89..a6d150f 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -216,8 +216,9 @@ void indentprint(const char *str, int indent) return; } - /* if we're not a tty, print without indenting */ - if(cols == 0) { + /* if we're not a tty, or our tty is not wide enough that wrapping even makes + * sense, print without indenting */ + if(cols == 0 || indent > cols) { printf("%s", str); return; } @@ -450,12 +451,16 @@ void list_display(const char *title, const alpm_list_t *list) if(!list) { printf("%s\n", _("None")); } else { - int cols; - const int maxcols = getcols(80); - for(i = list, cols = len; i; i = alpm_list_next(i)) { - char *str = alpm_list_getdata(i); + const int maxcols = getcols(0); + int cols = len; + const char *str = alpm_list_getdata(list); + printf("%s", str); + cols += string_length(str); + for(i = alpm_list_next(list), cols = len; i; i = alpm_list_next(i)) { + const char *str = alpm_list_getdata(i); int s = string_length(str); - if(cols + s + 2 >= maxcols) { + /* wrap only if we have enough usable column space */ + if(maxcols > len && cols + s + 2 >= maxcols) { int j; cols = len; printf("\n"); -- 1.7.5.2
On Mon, Jun 13, 2011 at 05:11:11PM -0500, Dan McGee wrote:
commit c1f742d7750a14 broke what was one of the tenants of out output- if piping pacman output somewhere else, we shouldn't ever try to line-wrap and indent print our output. This makes it easier for tools to use output from pacman -Ss, -Qs, -Qi, etc. list_display() unfortunately was given a default value of 80 rather than 0, so fix this.
Next, make some additional changes that ensure we don't insert an unnecessary blank line if for some crazy reason the indent level (such as on -Qi output) is greater than the number of columns. Accomplish this by printing the first item unconditionally as we do in list_display_linebreak().
Finally, teach indentprint to not wrap if the number of columns is less than the indent level, this prevents some forms of ridiculous output such as the following:
Install Date : Wed 08 Jun 2011 04:39:19 AM CDT
Signed-off-by: Dan McGee <dan@archlinux.org> ---
For maint. Dave, looking forward to your feedback. :)
-Dan
This works for me. I'd still like to see support for terminals with an unknown size. As we discussed, I'm fairly certain we can get away with using isatty() to figure this out and return a sane default (80) when the ioctl returns a 0 terminal width. d
src/pacman/util.c | 19 ++++++++++++------- 1 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/src/pacman/util.c b/src/pacman/util.c index 508cc89..a6d150f 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -216,8 +216,9 @@ void indentprint(const char *str, int indent) return; }
- /* if we're not a tty, print without indenting */ - if(cols == 0) { + /* if we're not a tty, or our tty is not wide enough that wrapping even makes + * sense, print without indenting */ + if(cols == 0 || indent > cols) { printf("%s", str); return; } @@ -450,12 +451,16 @@ void list_display(const char *title, const alpm_list_t *list) if(!list) { printf("%s\n", _("None")); } else { - int cols; - const int maxcols = getcols(80); - for(i = list, cols = len; i; i = alpm_list_next(i)) { - char *str = alpm_list_getdata(i); + const int maxcols = getcols(0); + int cols = len; + const char *str = alpm_list_getdata(list); + printf("%s", str); + cols += string_length(str); + for(i = alpm_list_next(list), cols = len; i; i = alpm_list_next(i)) { + const char *str = alpm_list_getdata(i); int s = string_length(str); - if(cols + s + 2 >= maxcols) { + /* wrap only if we have enough usable column space */ + if(maxcols > len && cols + s + 2 >= maxcols) { int j; cols = len; printf("\n"); -- 1.7.5.2
Fall back on the default passed to getcols when we encounter a device attached to stdout that reports itself as being a tty but which has no width. This is only (currently) known to affect serial consoles, which have a legitimate size of 80x24. Signed-off-by: Dave Reisner <d@falconindy.com> --- So this seems to solve the issues with serial consoles reporting crap widths but still keep real terminals happy. All calls to getcols now supply a default of 80 (which I guess makes sense), but we hardcode the width of 0 in the case of stdout not being a tty (pager/grep/sed/etc). Note that this only applies on top of Dan's previous patch which fixes list_display (after he broke it). d src/pacman/callback.c | 4 ++-- src/pacman/util.c | 16 +++++++++++----- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/pacman/callback.c b/src/pacman/callback.c index 4ac3b56..d956677 100644 --- a/src/pacman/callback.c +++ b/src/pacman/callback.c @@ -355,7 +355,7 @@ void cb_trans_progress(pmtransprog_t event, const char *pkgname, int percent, int len, wclen, wcwid, padwid; wchar_t *wcstr; - const int cols = getcols(0); + const int cols = getcols(80); if(config->noprogressbar || cols == 0) { return; @@ -504,7 +504,7 @@ void cb_dl_progress(const char *filename, off_t file_xfered, off_t file_total) const char *rate_label, *xfered_label; int file_percent = 0, total_percent = 0; - const int cols = getcols(0); + const int cols = getcols(80); if(config->noprogressbar || cols == 0 || file_total == -1) { if(file_xfered == 0) { diff --git a/src/pacman/util.c b/src/pacman/util.c index 9709011..a112d1c 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -118,18 +118,24 @@ static int flush_term_input(void) { /* gets the current screen column width */ int getcols(int def) { + int termwidth = -1; + + if(!isatty(fileno(stdout))) { + return 0; + } + #ifdef TIOCGSIZE struct ttysize win; if(ioctl(1, TIOCGSIZE, &win) == 0) { - return win.ts_cols; + termwidth = win.ts_cols; } #elif defined(TIOCGWINSZ) struct winsize win; if(ioctl(1, TIOCGWINSZ, &win) == 0) { - return win.ws_col; + termwidth = win.ws_col; } #endif - return def; + return termwidth <= 0 ? def : termwidth; } /* does the same thing as 'rm -rf' */ @@ -225,7 +231,7 @@ void indentprint(const char *str, int indent) wchar_t *wcstr; const wchar_t *p; int len, cidx; - const int cols = getcols(0); + const int cols = getcols(80); if(!str) { return; @@ -577,7 +583,7 @@ void list_display(const char *title, const alpm_list_t *list) if(!list) { printf("%s\n", _("None")); } else { - const int maxcols = getcols(0); + const int maxcols = getcols(80); int cols = len; const char *str = alpm_list_getdata(list); printf("%s", str); -- 1.7.5.4
On 06/14/2011 12:11 AM, Dan McGee wrote:
+ const char *str = alpm_list_getdata(list); + printf("%s", str); + cols += string_length(str); + for(i = alpm_list_next(list), cols = len; i; i = alpm_list_next(i)) {
cols is assigned and then overwritten in the next line?
+ const char *str = alpm_list_getdata(i); int s = string_length(str); - if(cols + s + 2>= maxcols) { + /* wrap only if we have enough usable column space */ + if(maxcols> len&& cols + s + 2>= maxcols) { int j; cols = len; printf("\n");
On Wed, Jun 15, 2011 at 4:03 AM, Jakob Gruber <jakob.gruber@gmail.com> wrote:
On 06/14/2011 12:11 AM, Dan McGee wrote:
+ const char *str = alpm_list_getdata(list); + printf("%s", str); + cols += string_length(str); + for(i = alpm_list_next(list), cols = len; i; i = alpm_list_next(i)) {
cols is assigned and then overwritten in the next line? Definite oversight here; removing the assignment from the for() should take care of any issue here, it appears.
+ const char *str = alpm_list_getdata(i); int s = string_length(str); - if(cols + s + 2>= maxcols) { + /* wrap only if we have enough usable column space */ + if(maxcols> len&& cols + s + 2>= maxcols) { int j; cols = len; printf("\n");
participants (4)
-
Dan McGee
-
Dan McGee
-
Dave Reisner
-
Jakob Gruber