[pacman-dev] word-wrapping when not outputting to a tty
I've noticed the following behaviour which makes it somewhat difficult to use -Qi/-Si/etc. in a pipeline: $ pacman -Si binutils ... Description : A set of programs to assemble and manipulate binary and object files $ pacman -Si binutils | cat Description : A set of programs to assemble and manipulate binary and object files I'd expect pacman not word-wrapping the output in the second case, as it breaks the line-oriented nature of data. This happens because getcols() defaults to 80 when !isatty(1), but I'd rather like it not to word-wrap at all in this case, as we're outputting to something like a file or a pipe. In the former case, some text processing tool could do better anyway, and the current behavior makes the latter case hardly useful at all. Does that make sense? Basically I'm suggesting (maybe) getting rid of the isatty() check in getcols() and adding one around word-wrapping code in indentprint() and list_display(). An attempt at implementing the proposal above can be found at http://rain.ifmo.ru/~olegfink/pacman-wrap.patch (sorry for not inlining, I use gmail). Maybe there should be more code shared between the two routines, e.g. getcols() could return -1 when word wrapping shouldn't be done. Any thoughts? Thanks, oleg
On Wed, 26 Aug 2009 23:46:04 +0400 Oleg Finkelshteyn <olegfink@gmail.com> wrote:
I've noticed the following behaviour which makes it somewhat difficult to use -Qi/-Si/etc. in a pipeline:
$ pacman -Si binutils ... Description : A set of programs to assemble and manipulate binary and object files $ pacman -Si binutils | cat Description : A set of programs to assemble and manipulate binary and object files
I'd expect pacman not word-wrapping the output in the second case, as it breaks the line-oriented nature of data. This happens because getcols() defaults to 80 when !isatty(1), but I'd rather like it not to word-wrap at all in this case, as we're outputting to something like a file or a pipe. In the former case, some text processing tool could do better anyway, and the current behavior makes the latter case hardly useful at all.
Does that make sense? Basically I'm suggesting (maybe) getting rid of the isatty() check in getcols() and adding one around word-wrapping code in indentprint() and list_display(). An attempt at implementing the proposal above can be found at http://rain.ifmo.ru/~olegfink/pacman-wrap.patch (sorry for not inlining, I use gmail). Maybe there should be more code shared between the two routines, e.g. getcols() could return -1 when word wrapping shouldn't be done.
Any thoughts?
Thanks, oleg
I've personally been bothered by this: it's probably related/same issue: pacman -Qi libsoup (...) Depends On : glib2>=2.20.4 gnutls>=2.8.1 libxml2>=2.7.3 libproxy>=0.2.3 sqlite3>=3.6.15 gconf>=2.26.0-3 (...) pacman -Qi libsoup | grep Depends Depends On : glib2>=2.20.4 gnutls>=2.8.1 libxml2>=2.7.3 libproxy>=0.2.3 Dieter
I've personally been bothered by this: it's probably related/same issue: pacman -Qi libsoup (...) Depends On : glib2>=2.20.4 gnutls>=2.8.1 libxml2>=2.7.3 libproxy>=0.2.3 sqlite3>=3.6.15 gconf>=2.26.0-3
My problem was with indentprint(), this comes from list_display(). My patch attempts to fix both. P.S.: Apparently gmail supports the current pacman behavior -- it word-wraps the output lines that shouldn't be. Sorry. P.P.S.: Seems the mails don't contain a Reply-To: -- is that normal? My mail client thinks it should reply to the sender rather than the list.
On Wed, Aug 26, 2009 at 2:59 PM, Oleg Finkelshteyn<olegfink@gmail.com> wrote:
P.P.S.: Seems the mails don't contain a Reply-To: -- is that normal? My mail client thinks it should reply to the sender rather than the list.
mailman settings look of, and it works for me in gmail
On Wed, Aug 26, 2009 at 3:09 PM, Aaron Griffin<aaronmgriffin@gmail.com> wrote:
On Wed, Aug 26, 2009 at 2:59 PM, Oleg Finkelshteyn<olegfink@gmail.com> wrote:
P.P.S.: Seems the mails don't contain a Reply-To: -- is that normal? My mail client thinks it should reply to the sender rather than the list.
mailman settings look of, and it works for me in gmail mailman settings look *OK*
mailman settings look of, and it works for me in gmail
turns out because of the Cc: to me in Dieter's message I got two copies and gmail chose to display the private one without Reply-to:. Sorry for the noise.
On Wed, Aug 26, 2009 at 9:46 PM, Oleg Finkelshteyn<olegfink@gmail.com> wrote:
I've noticed the following behaviour which makes it somewhat difficult to use -Qi/-Si/etc. in a pipeline:
$ pacman -Si binutils ... Description : A set of programs to assemble and manipulate binary and object files $ pacman -Si binutils | cat Description : A set of programs to assemble and manipulate binary and object files
I'd expect pacman not word-wrapping the output in the second case, as it breaks the line-oriented nature of data. This happens because getcols() defaults to 80 when !isatty(1), but I'd rather like it not to word-wrap at all in this case, as we're outputting to something like a file or a pipe. In the former case, some text processing tool could do better anyway, and the current behavior makes the latter case hardly useful at all.
Does that make sense? Basically I'm suggesting (maybe) getting rid of the isatty() check in getcols() and adding one around word-wrapping code in indentprint() and list_display(). An attempt at implementing the proposal above can be found at http://rain.ifmo.ru/~olegfink/pacman-wrap.patch (sorry for not inlining, I use gmail). Maybe there should be more code shared between the two routines, e.g. getcols() could return -1 when word wrapping shouldn't be done.
Any thoughts?
I am fine with that patch. I would also be happy with "getcols() could return -1 when word wrapping shouldn't be done." Not sure which one is better. The latter would remove the need for any isatty call.
From: Oleg Finkelshteyn <olegfink@gmail.com> for example when we are not in a tty, there is no point in wrapping the output. this actually makes the job harder for scripts. $ pacman -Si binutils | grep Desc Description : A set of programs to assemble and manipulate binary and the description was cut because the rest was on the following line. Signed-off-by: Xavier Chantry <shiningxc@gmail.com> --- src/pacman/util.c | 45 ++++++++++++++++++--------------------------- 1 files changed, 18 insertions(+), 27 deletions(-) diff --git a/src/pacman/util.c b/src/pacman/util.c index 9c4b797..3c3379a 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -95,34 +95,18 @@ int needs_root(void) /* gets the current screen column width */ int getcols(void) { - if(!isatty(1)) { - /* We will default to 80 columns if we're not a tty - * this seems a fairly standard file width. - */ - return 80; - } else { #ifdef TIOCGSIZE - struct ttysize win; - if(ioctl(1, TIOCGSIZE, &win) == 0) { - return win.ts_cols; - } -#elif defined(TIOCGWINSZ) - struct winsize win; - if(ioctl(1, TIOCGWINSZ, &win) == 0) { - return win.ws_col; - } -#endif - /* If we can't figure anything out, we'll just assume 80 columns */ - /* TODO any problems caused by this assumption? */ - return 80; + struct ttysize win; + if(ioctl(1, TIOCGSIZE, &win) == 0) { + return win.ts_cols; } - /* Original envvar way - prone to display issues - const char *cenv = getenv("COLUMNS"); - if(cenv != NULL) { - return atoi(cenv); +#elif defined(TIOCGWINSZ) + struct winsize win; + if(ioctl(1, TIOCGWINSZ, &win) == 0) { + return win.ws_col; } - return -1; - */ +#endif + return 0; } /* does the same thing as 'mkdir -p' */ @@ -266,6 +250,14 @@ void indentprint(const char *str, int indent) return; } + cols = getcols(); + + /* if we're a tty, just plain print the string */ + if(cols == 0) { + fputs(str, stdout); + return; + } + len = strlen(str) + 1; wcstr = calloc(len, sizeof(wchar_t)); len = mbstowcs(wcstr, str, len); @@ -275,7 +267,6 @@ void indentprint(const char *str, int indent) if(!p) { return; } - cols = getcols(); while(*p) { if(*p == L' ') { @@ -486,7 +477,7 @@ void list_display(const char *title, const alpm_list_t *list) /* two additional spaces are added to the length */ s += 2; int maxcols = getcols(); - if(s + cols > maxcols) { + if(s + cols > maxcols && maxcols > 0) { int j; cols = len; printf("\n"); -- 1.6.4.1
On Thu, Aug 27, 2009 at 12:45 AM, Xavier Chantry<shiningxc@gmail.com> wrote:
From: Oleg Finkelshteyn <olegfink@gmail.com>
for example when we are not in a tty, there is no point in wrapping the output. this actually makes the job harder for scripts.
$ pacman -Si binutils | grep Desc Description : A set of programs to assemble and manipulate binary and
the description was cut because the rest was on the following line.
Signed-off-by: Xavier Chantry <shiningxc@gmail.com> --- src/pacman/util.c | 45 ++++++++++++++++++--------------------------- 1 files changed, 18 insertions(+), 27 deletions(-)
diff --git a/src/pacman/util.c b/src/pacman/util.c index 9c4b797..3c3379a 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -95,34 +95,18 @@ int needs_root(void) /* gets the current screen column width */ int getcols(void) { - if(!isatty(1)) { - /* We will default to 80 columns if we're not a tty - * this seems a fairly standard file width. - */ - return 80; - } else { #ifdef TIOCGSIZE - struct ttysize win; - if(ioctl(1, TIOCGSIZE, &win) == 0) { - return win.ts_cols; - } -#elif defined(TIOCGWINSZ) - struct winsize win; - if(ioctl(1, TIOCGWINSZ, &win) == 0) { - return win.ws_col; - } -#endif - /* If we can't figure anything out, we'll just assume 80 columns */ - /* TODO any problems caused by this assumption? */ - return 80; + struct ttysize win; + if(ioctl(1, TIOCGSIZE, &win) == 0) { + return win.ts_cols; } - /* Original envvar way - prone to display issues - const char *cenv = getenv("COLUMNS"); - if(cenv != NULL) { - return atoi(cenv); +#elif defined(TIOCGWINSZ) + struct winsize win; + if(ioctl(1, TIOCGWINSZ, &win) == 0) { + return win.ws_col; } - return -1; - */ +#endif + return 0; }
this patch looks hard to read, but we basically just removed the first if(!isatty(1)) check, so all the rest was re-indented.
On Thu, Aug 27, 2009 at 12:48:09AM +0200, Xavier wrote:
On Thu, Aug 27, 2009 at 12:45 AM, Xavier Chantry<shiningxc@gmail.com> wrote:
From: Oleg Finkelshteyn <olegfink@gmail.com>
for example when we are not in a tty, there is no point in wrapping the output. this actually makes the job harder for scripts.
$ pacman -Si binutils | grep Desc Description : A set of programs to assemble and manipulate binary and
the description was cut because the rest was on the following line.
Signed-off-by: Xavier Chantry <shiningxc@gmail.com> --- src/pacman/util.c | 45 ++++++++++++++++++--------------------------- 1 files changed, 18 insertions(+), 27 deletions(-)
diff --git a/src/pacman/util.c b/src/pacman/util.c index 9c4b797..3c3379a 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -95,34 +95,18 @@ int needs_root(void) /* gets the current screen column width */ int getcols(void) { - if(!isatty(1)) { - /* We will default to 80 columns if we're not a tty - * this seems a fairly standard file width. - */ - return 80; - } else { #ifdef TIOCGSIZE - struct ttysize win; - if(ioctl(1, TIOCGSIZE, &win) == 0) { - return win.ts_cols; - } -#elif defined(TIOCGWINSZ) - struct winsize win; - if(ioctl(1, TIOCGWINSZ, &win) == 0) { - return win.ws_col; - } -#endif - /* If we can't figure anything out, we'll just assume 80 columns */ - /* TODO any problems caused by this assumption? */ - return 80; + struct ttysize win; + if(ioctl(1, TIOCGSIZE, &win) == 0) { + return win.ts_cols; } - /* Original envvar way - prone to display issues - const char *cenv = getenv("COLUMNS"); - if(cenv != NULL) { - return atoi(cenv); +#elif defined(TIOCGWINSZ) + struct winsize win; + if(ioctl(1, TIOCGWINSZ, &win) == 0) { + return win.ws_col; } - return -1; - */ +#endif + return 0; }
this patch looks hard to read, but we basically just removed the first if(!isatty(1)) check, so all the rest was re-indented.
May I ask , when this patch will be pushed ?
On Wed, Oct 14, 2009 at 3:38 PM, <Nezmer@allurelinux.org> wrote:
May I ask , when this patch will be pushed ?
One month and 8 days ago : http://projects.archlinux.org/?p=pacman.git;a=commit;h=5dbd00faf7a27866ffc2a... I doubt it was pushed to maint though. Only to master. which means it will only be in 3.4
On Wed, Oct 14, 2009 at 03:55:06PM +0200, Xavier wrote:
On Wed, Oct 14, 2009 at 3:38 PM, <Nezmer@allurelinux.org> wrote:
May I ask , when this patch will be pushed ?
One month and 8 days ago : http://projects.archlinux.org/?p=pacman.git;a=commit;h=5dbd00faf7a27866ffc2a...
I doubt it was pushed to maint though. Only to master. which means it will only be in 3.4 yeah , I meant in a release. Thank you for the info.
participants (6)
-
Aaron Griffin
-
Dieter Plaetinck
-
Nezmer＠allurelinux.org
-
Oleg Finkelshteyn
-
Xavier
-
Xavier Chantry