all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* [bug#30053] [PATCH 1/3] Improve appearance of tabular output.
@ 2018-01-09 22:34 Steve Sprang
  2018-01-09 22:37 ` [bug#30053] [PATCH 2/3] " Steve Sprang
                   ` (2 more replies)
  0 siblings, 3 replies; 23+ messages in thread
From: Steve Sprang @ 2018-01-09 22:34 UTC (permalink / raw)
  To: 30053

[-- Attachment #1: Type: text/plain, Size: 354 bytes --]

I noticed when listing installed or available packages that the output
is often pretty jumbled up because columns in each row have an
inconsistent width.

This series of patches adds a new procedure for printing tabular data
(pretty-print-table) and modifies the code for --list-installed,
--list-available, and --list-generations to utilize it.

-Steve

[-- Attachment #2: 0001-utils-Add-a-procedure-for-pretty-printing-tabular-da.patch --]
[-- Type: text/x-patch, Size: 1724 bytes --]

From 09a6bbb1a8d5d2855cdee06b5937dc3e95b2f401 Mon Sep 17 00:00:00 2001
From: Steve Sprang <scs@stevesprang.com>
Date: Tue, 9 Jan 2018 14:00:11 -0800
Subject: [PATCH 1/3] utils: Add a procedure for pretty printing tabular data.

* guix/utils.scm (pretty-print-table): New procedure.
---
 guix/utils.scm | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/guix/utils.scm b/guix/utils.scm
index 92e45de61..cf1d88d21 100644
--- a/guix/utils.scm
+++ b/guix/utils.scm
@@ -46,7 +46,9 @@
   #:use-module ((ice-9 iconv) #:prefix iconv:)
   #:use-module (system foreign)
   #:re-export (memoize)         ; for backwards compatibility
-  #:export (strip-keyword-arguments
+  #:export (pretty-print-table
+
+            strip-keyword-arguments
             default-keyword-arguments
             substitute-keyword-arguments
             ensure-keyword-arguments
@@ -299,6 +301,24 @@ This procedure returns #t on success."
             #t))))))
 
 \f
+;;;
+;;; Prettified output.
+;;;
+
+(define (pretty-print-table rows)
+  "Print ROWS in neat columns.  All rows should be lists of strings and each
+row should have the same length."
+  (let* ((num-cols   (if (null? rows) 0 (length (car rows))))
+         (col-widths (fold (lambda (row maximums)
+                             (map max (map string-length row) maximums))
+                           ;; Initial max width is 0 for each column.
+                           (make-list num-cols 0)
+                           rows))
+         (col-fmts   (map (cut format #f "~~~da" <>) col-widths))
+         (fmt        (string-join col-fmts "~/")))
+    (map (cut format #t "~?~%" fmt <>) rows)))
+
+\f
 ;;;
 ;;; Keyword arguments.
 ;;;
-- 
2.15.1


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

* [bug#30053] [PATCH 2/3] Improve appearance of tabular output.
  2018-01-09 22:34 [bug#30053] [PATCH 1/3] Improve appearance of tabular output Steve Sprang
@ 2018-01-09 22:37 ` Steve Sprang
  2018-01-09 22:37 ` [bug#30053] [PATCH 3/3] " Steve Sprang
  2018-01-11 21:32 ` [bug#30053] [PATCH 1/3] " Ludovic Courtès
  2 siblings, 0 replies; 23+ messages in thread
From: Steve Sprang @ 2018-01-09 22:37 UTC (permalink / raw)
  To: 30053

[-- Attachment #1: Type: text/plain, Size: 1 bytes --]



[-- Attachment #2: 0002-package-Improve-output-appearance-when-listing-packa.patch --]
[-- Type: text/x-patch, Size: 3042 bytes --]

From c3454179249c84651f50fdfb88950b66f5760923 Mon Sep 17 00:00:00 2001
From: Steve Sprang <scs@stevesprang.com>
Date: Tue, 9 Jan 2018 14:10:04 -0800
Subject: [PATCH 2/3] package: Improve output appearance when listing packages.

* guix/scripts/package.scm (process-query): Use pretty-print-table when listing installed and available packages.
---
 guix/scripts/package.scm | 37 ++++++++++++++++++-------------------
 1 file changed, 18 insertions(+), 19 deletions(-)

diff --git a/guix/scripts/package.scm b/guix/scripts/package.scm
index 617e102d9..ced85f850 100644
--- a/guix/scripts/package.scm
+++ b/guix/scripts/package.scm
@@ -723,15 +723,14 @@ processed, #f otherwise."
               (manifest  (profile-manifest profile))
               (installed (manifest-entries manifest)))
          (leave-on-EPIPE
-          (for-each (match-lambda
-                      (($ <manifest-entry> name version output path _)
-                       (when (or (not regexp)
-                                 (regexp-exec regexp name))
-                         (format #t "~a\t~a\t~a\t~a~%"
-                                 name (or version "?") output path))))
-
-                    ;; Show most recently installed packages last.
-                    (reverse installed)))
+          (let ((rows (filter-map
+                       (match-lambda
+                         (($ <manifest-entry> name version output path _)
+                          (and (regexp-exec regexp name)
+                               (list name (or version "?") output path))))
+                       installed)))
+            ;; Show most recently installed packages last.
+            (pretty-print-table (reverse rows))))
          #t))
 
       (('list-available regexp)
@@ -749,16 +748,16 @@ processed, #f otherwise."
                                   r)))
                           '())))
          (leave-on-EPIPE
-          (for-each (lambda (p)
-                      (format #t "~a\t~a\t~a\t~a~%"
-                              (package-name p)
-                              (package-version p)
-                              (string-join (package-outputs p) ",")
-                              (location->string (package-location p))))
-                    (sort available
-                          (lambda (p1 p2)
-                            (string<? (package-name p1)
-                                      (package-name p2))))))
+          (let ((rows (map (lambda (p)
+                             (list (package-name p)
+                                   (package-version p)
+                                   (string-join (package-outputs p) ",")
+                                   (location->string (package-location p))))
+                           (sort available
+                                 (lambda (p1 p2)
+                                   (string<? (package-name p1)
+                                             (package-name p2)))))))
+            (pretty-print-table rows)))
          #t))
 
       (('search _)
-- 
2.15.1


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

* [bug#30053] [PATCH 3/3] Improve appearance of tabular output.
  2018-01-09 22:34 [bug#30053] [PATCH 1/3] Improve appearance of tabular output Steve Sprang
  2018-01-09 22:37 ` [bug#30053] [PATCH 2/3] " Steve Sprang
@ 2018-01-09 22:37 ` Steve Sprang
  2018-01-11 21:32 ` [bug#30053] [PATCH 1/3] " Ludovic Courtès
  2 siblings, 0 replies; 23+ messages in thread
From: Steve Sprang @ 2018-01-09 22:37 UTC (permalink / raw)
  To: 30053

[-- Attachment #1: Type: text/plain, Size: 1 bytes --]



[-- Attachment #2: 0003-ui-Improve-output-appearance-when-listing-generation.patch --]
[-- Type: text/x-patch, Size: 2898 bytes --]

From a4b67e9255ac50750c8f82f6aee8ebc35f8ba2bd Mon Sep 17 00:00:00 2001
From: Steve Sprang <scs@stevesprang.com>
Date: Tue, 9 Jan 2018 14:20:12 -0800
Subject: [PATCH 3/3] ui: Improve output appearance when listing generations.

* guix/ui.scm (display-profile-content-diff): Use pretty-print-table to format output.
(display-profile-content): Likewise.
---
 guix/ui.scm | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/guix/ui.scm b/guix/ui.scm
index 895179744..32f618f33 100644
--- a/guix/ui.scm
+++ b/guix/ui.scm
@@ -1334,10 +1334,10 @@ DURATION-RELATION with the current time."
   (define (equal-entry? first second)
     (string= (manifest-entry-item first) (manifest-entry-item second)))
 
-  (define (display-entry entry prefix)
+  (define (make-row entry prefix)
     (match entry
       (($ <manifest-entry> name version output location _)
-       (format #t " ~a ~a\t~a\t~a\t~a~%" prefix name version output location))))
+       (list (format #f " ~a ~a" prefix name) version output location))))
 
   (define (list-entries number)
     (manifest-entries (profile-manifest (generation-file-name profile number))))
@@ -1348,8 +1348,8 @@ DURATION-RELATION with the current time."
                   equal-entry? (list-entries new) (list-entries old)))
           (removed (lset-difference
                     equal-entry? (list-entries old) (list-entries new))))
-      (for-each (cut display-entry <> "+") added)
-      (for-each (cut display-entry <> "-") removed)
+      (pretty-print-table (append (map (cut make-row <> "+") added)
+                                  (map (cut make-row <> "-") removed)))
       (newline)))
 
   (display-diff profile gen1 gen2))
@@ -1357,15 +1357,17 @@ DURATION-RELATION with the current time."
 (define (display-profile-content profile number)
   "Display the packages in PROFILE, generation NUMBER, in a human-readable
 way."
-  (for-each (match-lambda
-              (($ <manifest-entry> name version output location _)
-               (format #t "  ~a\t~a\t~a\t~a~%"
-                       name version output location)))
-
-            ;; Show most recently installed packages last.
-            (reverse
-             (manifest-entries
-              (profile-manifest (generation-file-name profile number))))))
+
+  (define entry->row
+    (match-lambda
+      (($ <manifest-entry> name version output location _)
+       (list (string-append "  " name) version output location))))
+
+  (let* ((manifest (profile-manifest (generation-file-name profile number)))
+         (entries  (manifest-entries manifest))
+         (rows     (map entry->row entries)))
+    ;; Show most recently installed packages last.
+    (pretty-print-table (reverse rows))))
 
 (define (display-generation-change previous current)
   (format #t (G_ "switched from generation ~a to ~a~%") previous current))
-- 
2.15.1


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

* [bug#30053] [PATCH 1/3] Improve appearance of tabular output.
  2018-01-09 22:34 [bug#30053] [PATCH 1/3] Improve appearance of tabular output Steve Sprang
  2018-01-09 22:37 ` [bug#30053] [PATCH 2/3] " Steve Sprang
  2018-01-09 22:37 ` [bug#30053] [PATCH 3/3] " Steve Sprang
@ 2018-01-11 21:32 ` Ludovic Courtès
  2018-01-11 23:32   ` Steve Sprang
  2018-01-12 14:56   ` Danny Milosavljevic
  2 siblings, 2 replies; 23+ messages in thread
From: Ludovic Courtès @ 2018-01-11 21:32 UTC (permalink / raw)
  To: Steve Sprang; +Cc: 30053

Hello Steve,

Long time no see!  ;-)

Steve Sprang <steve.sprang@gmail.com> skribis:

> I noticed when listing installed or available packages that the output
> is often pretty jumbled up because columns in each row have an
> inconsistent width.
>
> This series of patches adds a new procedure for printing tabular data
> (pretty-print-table) and modifies the code for --list-installed,
> --list-available, and --list-generations to utilize it.

I have a disappointing explanation I’m afraid: the reason columns look
this way is because they are tab-separated, which in turn makes it easy
to filter with ‘cut’:

--8<---------------cut here---------------start------------->8---
$ guix package -A | cut -f1 | head
0ad
0ad-data
0xffff
4store
4ti2
a2ps
aalib
abbaye
abc
abcde
--8<---------------cut here---------------end--------------->8---

An example from the manual (info "(guix) Invoking guix build"):

     guix build --quiet --keep-going \
       `guix package -A | cut -f1,2 --output-delimiter=@`

The idea was to have this shell-scripting-friendly format, and to
provide fancier output in other commands, such as --search (which is in
fact script-friendly as well thanks to recutils).

Silly? Awesome? Ugly? What do people think?  :-)

Thank you,
Ludo’.

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

* [bug#30053] [PATCH 1/3] Improve appearance of tabular output.
  2018-01-11 21:32 ` [bug#30053] [PATCH 1/3] " Ludovic Courtès
@ 2018-01-11 23:32   ` Steve Sprang
  2018-01-12 13:28     ` Roel Janssen
  2018-01-12 14:56   ` Danny Milosavljevic
  1 sibling, 1 reply; 23+ messages in thread
From: Steve Sprang @ 2018-01-11 23:32 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 30053

Hi Ludovic,

On Thu, Jan 11, 2018 at 1:32 PM, Ludovic Courtès <ludo@gnu.org> wrote:
> Long time no see!  ;-)

Yeah, it's been a while!

> I have a disappointing explanation I’m afraid: the reason columns look
> this way is because they are tab-separated, which in turn makes it easy
> to filter with ‘cut’:
>
> --8<---------------cut here---------------start------------->8---
> $ guix package -A | cut -f1 | head
> 0ad
> 0ad-data
> 0xffff
> 4store
> 4ti2
> a2ps
> aalib
> abbaye
> abc
> abcde
> --8<---------------cut here---------------end--------------->8---

I'm still inserting a tab between columns, so I believe 'cut' still
works as expected in this case. Initially, I was separating columns
with a few spaces, but that broke some of the tests that were relying
on cut, so I switched back to tab.

> An example from the manual (info "(guix) Invoking guix build"):
>
>      guix build --quiet --keep-going \
>        `guix package -A | cut -f1,2 --output-delimiter=@`

Argh, this use case fails because of the extra inserted whitespace.

> The idea was to have this shell-scripting-friendly format, and to
> provide fancier output in other commands, such as --search (which is in
> fact script-friendly as well thanks to recutils).
>
> Silly? Awesome? Ugly? What do people think?  :-)

Another potential drawback of this patch is that it tends to make
output lines longer than before. This might make line-wrapping less
pleasant when using smaller terminal windows/screens.

-Steve

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

* [bug#30053] [PATCH 1/3] Improve appearance of tabular output.
  2018-01-11 23:32   ` Steve Sprang
@ 2018-01-12 13:28     ` Roel Janssen
  2018-01-13 19:59       ` Steve Sprang
  0 siblings, 1 reply; 23+ messages in thread
From: Roel Janssen @ 2018-01-12 13:28 UTC (permalink / raw)
  To: Steve Sprang; +Cc: 30053


Steve Sprang writes:

> Hi Ludovic,
>
> On Thu, Jan 11, 2018 at 1:32 PM, Ludovic Courtès <ludo@gnu.org> wrote:
>> Long time no see!  ;-)
>
> Yeah, it's been a while!
>
>> I have a disappointing explanation I’m afraid: the reason columns look
>> this way is because they are tab-separated, which in turn makes it easy
>> to filter with ‘cut’:
>>
>> --8<---------------cut here---------------start------------->8---
>> $ guix package -A | cut -f1 | head
>> 0ad
>> 0ad-data
>> 0xffff
>> 4store
>> 4ti2
>> a2ps
>> aalib
>> abbaye
>> abc
>> abcde
>> --8<---------------cut here---------------end--------------->8---
>
> I'm still inserting a tab between columns, so I believe 'cut' still
> works as expected in this case. Initially, I was separating columns
> with a few spaces, but that broke some of the tests that were relying
> on cut, so I switched back to tab.
>
>> An example from the manual (info "(guix) Invoking guix build"):
>>
>>      guix build --quiet --keep-going \
>>        `guix package -A | cut -f1,2 --output-delimiter=@`
>
> Argh, this use case fails because of the extra inserted whitespace.
>
>> The idea was to have this shell-scripting-friendly format, and to
>> provide fancier output in other commands, such as --search (which is in
>> fact script-friendly as well thanks to recutils).
>>
>> Silly? Awesome? Ugly? What do people think?  :-)
>
> Another potential drawback of this patch is that it tends to make
> output lines longer than before. This might make line-wrapping less
> pleasant when using smaller terminal windows/screens.
>
> -Steve

If we use GNU awk instead of cut, I think any whitespace will work:
  $ guix package -A | awk '{ print $1 "@" $2 }'

And then we can optimize the output reading experience for our users
instead of for the 'cut' program.

Kind regards,
Roel Janssen

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

* [bug#30053] [PATCH 1/3] Improve appearance of tabular output.
  2018-01-11 21:32 ` [bug#30053] [PATCH 1/3] " Ludovic Courtès
  2018-01-11 23:32   ` Steve Sprang
@ 2018-01-12 14:56   ` Danny Milosavljevic
  2018-01-12 15:26     ` Leo Famulari
  1 sibling, 1 reply; 23+ messages in thread
From: Danny Milosavljevic @ 2018-01-12 14:56 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 30053, Steve Sprang

Hi Ludo,
Hi Steve,

terminals support setting tab stops.  The user could (and arguably should) just set them (see tabs(1).  Example invocation: "tabs 1,20,25").

> I have a disappointing explanation I’m afraid: the reason columns look
> this way is because they are tab-separated, which in turn makes it easy
> to filter with ‘cut’:

Also, they are columns in a TABle.

There's also "column -t" which one can pipe the output to, which will take care of autosizing the columns. 

It's typical of UNIX tools that they prefer machine readability to usability.  There are always small tools like the ones above one can use (either before the invocation or after the invocation) to make output more usable.

That said, Steve even retains the tabs, so both use cases would be supported.  The only cost is that with the patch there's a lot of whitespace printed in columns that's actually not in the database at all.  But that's OK I think.

Also, "guix package -A" (even before the patch) eats up all my 8 GB of RAM on guix master and then my computer hangs.  What's up with that? (I tried it 3 times now - it's reproducible) O_o

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

* [bug#30053] [PATCH 1/3] Improve appearance of tabular output.
  2018-01-12 14:56   ` Danny Milosavljevic
@ 2018-01-12 15:26     ` Leo Famulari
  2018-01-12 15:50       ` bug#30087: "guix package -A" hangs with attached package set Danny Milosavljevic
  2018-01-13 13:47       ` [bug#30053] [PATCH 1/3] Improve appearance of tabular output Ludovic Courtès
  0 siblings, 2 replies; 23+ messages in thread
From: Leo Famulari @ 2018-01-12 15:26 UTC (permalink / raw)
  To: Danny Milosavljevic; +Cc: 30053, Steve Sprang

[-- Attachment #1: Type: text/plain, Size: 360 bytes --]

On Fri, Jan 12, 2018 at 03:56:12PM +0100, Danny Milosavljevic wrote:
> Also, "guix package -A" (even before the patch) eats up all my 8 GB of
> RAM on guix master and then my computer hangs.  What's up with that?
> (I tried it 3 times now - it's reproducible) O_o

I can't reproduce this with a recent Guix. Are you using any custom
packages or modifications?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* bug#30087: "guix package -A" hangs with attached package set
  2018-01-12 15:26     ` Leo Famulari
@ 2018-01-12 15:50       ` Danny Milosavljevic
  2020-12-03  0:10         ` zimoun
  2018-01-13 13:47       ` [bug#30053] [PATCH 1/3] Improve appearance of tabular output Ludovic Courtès
  1 sibling, 1 reply; 23+ messages in thread
From: Danny Milosavljevic @ 2018-01-12 15:50 UTC (permalink / raw)
  To: Leo Famulari; +Cc: 30087

[-- Attachment #1: Type: text/plain, Size: 600 bytes --]

Hi Leo,

On Fri, 12 Jan 2018 07:26:31 -0800
Leo Famulari <leo@famulari.name> wrote:

> I can't reproduce this with a recent Guix. Are you using any custom
> packages or modifications?

Yeah, sorry, I checked it now.  It's because of packages in my GUIX_PACKAGE_PATH.  When I move them away, "guix package -A" works fine.

Attached the offending directory from inside my GUIX_PACKAGE_PATH.

Warning!  If you decide to extract the attachment inside your GUIX_PACKAGE_PATH, be sure to save your work!  This hangs your computer.

I think it should still not happen.  Hence, I opened a new bugreport now.

[-- Attachment #2: wip.tar.gz --]
[-- Type: application/gzip, Size: 160066 bytes --]

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

* [bug#30053] [PATCH 1/3] Improve appearance of tabular output.
  2018-01-12 15:26     ` Leo Famulari
  2018-01-12 15:50       ` bug#30087: "guix package -A" hangs with attached package set Danny Milosavljevic
@ 2018-01-13 13:47       ` Ludovic Courtès
  1 sibling, 0 replies; 23+ messages in thread
From: Ludovic Courtès @ 2018-01-13 13:47 UTC (permalink / raw)
  To: Leo Famulari; +Cc: 30053, Steve Sprang

Leo Famulari <leo@famulari.name> skribis:

> On Fri, Jan 12, 2018 at 03:56:12PM +0100, Danny Milosavljevic wrote:
>> Also, "guix package -A" (even before the patch) eats up all my 8 GB of
>> RAM on guix master and then my computer hangs.  What's up with that?
>> (I tried it 3 times now - it's reproducible) O_o
>
> I can't reproduce this with a recent Guix. Are you using any custom
> packages or modifications?

Yeah you most likely have something fishy in $GUIX_PACKAGE_PATH.

Ludo’.

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

* [bug#30053] [PATCH 1/3] Improve appearance of tabular output.
  2018-01-12 13:28     ` Roel Janssen
@ 2018-01-13 19:59       ` Steve Sprang
  2018-01-16 14:16         ` Ludovic Courtès
  0 siblings, 1 reply; 23+ messages in thread
From: Steve Sprang @ 2018-01-13 19:59 UTC (permalink / raw)
  To: Roel Janssen; +Cc: 30053

On Fri, Jan 12, 2018 at 5:28 AM, Roel Janssen <roel@gnu.org> wrote:
> If we use GNU awk instead of cut, I think any whitespace will work:
>   $ guix package -A | awk '{ print $1 "@" $2 }'
>
> And then we can optimize the output reading experience for our users
> instead of for the 'cut' program.

I like this proposal, unless there is a strong reason to prefer 'cut'?

We would obviously need to update relevant scripts and documentation.
It might also break any user scripts relying on the current behavior.

Since awk can more flexibly separate fields (versus cut's single
character delimiter) I could modify this patch to separate columns
with one or two spaces instead of tabs. This generally produces a
table with shorter line lengths and a neater presentation.

-Steve

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

* [bug#30053] [PATCH 1/3] Improve appearance of tabular output.
  2018-01-13 19:59       ` Steve Sprang
@ 2018-01-16 14:16         ` Ludovic Courtès
  2018-01-16 23:56           ` Steve Sprang
  2021-07-15  5:39           ` Maxim Cournoyer
  0 siblings, 2 replies; 23+ messages in thread
From: Ludovic Courtès @ 2018-01-16 14:16 UTC (permalink / raw)
  To: Steve Sprang; +Cc: 30053

Steve Sprang <steve.sprang@gmail.com> skribis:

> On Fri, Jan 12, 2018 at 5:28 AM, Roel Janssen <roel@gnu.org> wrote:
>> If we use GNU awk instead of cut, I think any whitespace will work:
>>   $ guix package -A | awk '{ print $1 "@" $2 }'
>>
>> And then we can optimize the output reading experience for our users
>> instead of for the 'cut' program.
>
> I like this proposal, unless there is a strong reason to prefer 'cut'?

Again a matter of taste, but ‘cut’ looks to me both easier and simpler
than awk (since it’s a full language).

But anyway, as Danny write, if your patches retain tabs (in addition to
spaces), presumably it’s OK even for those of us who prefer ‘cut’,
right?

Thanks,
Ludo’.

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

* [bug#30053] [PATCH 1/3] Improve appearance of tabular output.
  2018-01-16 14:16         ` Ludovic Courtès
@ 2018-01-16 23:56           ` Steve Sprang
  2021-07-15  5:39           ` Maxim Cournoyer
  1 sibling, 0 replies; 23+ messages in thread
From: Steve Sprang @ 2018-01-16 23:56 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 30053

On Tue, Jan 16, 2018 at 6:16 AM, Ludovic Courtès <ludo@gnu.org> wrote:

> Again a matter of taste, but ‘cut’ looks to me both easier and simpler
> than awk (since it’s a full language).
>
> But anyway, as Danny write, if your patches retain tabs (in addition to
> spaces), presumably it’s OK even for those of us who prefer ‘cut’,
> right?

If the tab is retained I think most uses of 'cut' keep working. The
only exception I'm aware of is this:

$ guix build --quiet --keep-going \
        `guix package -A | cut -f1,2 --output-delimiter=@`

Since 'cut' splits on the tab you end up retaining the padding spaces
from field 1 resulting in, say, "guile      @2.2.3" instead of the
desired "guile@2.2.3".

-Steve

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

* bug#30087: "guix package -A" hangs with attached package set
  2018-01-12 15:50       ` bug#30087: "guix package -A" hangs with attached package set Danny Milosavljevic
@ 2020-12-03  0:10         ` zimoun
  2020-12-22 16:12           ` zimoun
  0 siblings, 1 reply; 23+ messages in thread
From: zimoun @ 2020-12-03  0:10 UTC (permalink / raw)
  To: Danny Milosavljevic; +Cc: steve.sprang, 30087

Hi Danny,

This old bug #30087 is about something specific to one of your use
cases.

   <http://issues.guix.gnu.org/issue/30087>

On Fri, 12 Jan 2018 at 16:50, Danny Milosavljevic <dannym@scratchpost.org> wrote:
> On Fri, 12 Jan 2018 07:26:31 -0800
> Leo Famulari <leo@famulari.name> wrote:
>
>> I can't reproduce this with a recent Guix. Are you using any custom
>> packages or modifications?
>
> Yeah, sorry, I checked it now.  It's because of packages in my GUIX_PACKAGE_PATH.  When I move them away, "guix package -A" works fine.
>
> Attached the offending directory from inside my GUIX_PACKAGE_PATH.
>
> Warning!  If you decide to extract the attachment inside your GUIX_PACKAGE_PATH, be sure to save your work!  This hangs your computer.
>
> I think it should still not happen.  Hence, I opened a new bugreport now.

Is it still relevant?  If yes, could you provide more information to
tackle it?  For example, more context or recipe to reproduce it.  If no,
feel free to close it.


All the best,
simon




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

* bug#30087: "guix package -A" hangs with attached package set
  2020-12-03  0:10         ` zimoun
@ 2020-12-22 16:12           ` zimoun
  2021-01-11 12:29             ` zimoun
  0 siblings, 1 reply; 23+ messages in thread
From: zimoun @ 2020-12-22 16:12 UTC (permalink / raw)
  To: Danny Milosavljevic; +Cc: steve.sprang, 30087

Hi Danny,

On Thu, 03 Dec 2020 at 01:10, zimoun <zimon.toutoune@gmail.com> wrote:

> This old bug #30087 is about something specific to one of your use
> cases.
>
>    <http://issues.guix.gnu.org/issue/30087>
>
> On Fri, 12 Jan 2018 at 16:50, Danny Milosavljevic <dannym@scratchpost.org> wrote:
>> On Fri, 12 Jan 2018 07:26:31 -0800
>> Leo Famulari <leo@famulari.name> wrote:
>>
>>> I can't reproduce this with a recent Guix. Are you using any custom
>>> packages or modifications?
>>
>> Yeah, sorry, I checked it now.  It's because of packages in my GUIX_PACKAGE_PATH.  When I move them away, "guix package -A" works fine.
>>
>> Attached the offending directory from inside my GUIX_PACKAGE_PATH.
>>
>> Warning!  If you decide to extract the attachment inside your GUIX_PACKAGE_PATH, be sure to save your work!  This hangs your computer.
>>
>> I think it should still not happen.  Hence, I opened a new bugreport now.
>
> Is it still relevant?  If yes, could you provide more information to
> tackle it?  For example, more context or recipe to reproduce it.  If no,
> feel free to close it.

If no news in the coming days about the relevance of this bug, I will
took the liberty to close it.

Cheers,
simon




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

* bug#30087: "guix package -A" hangs with attached package set
  2020-12-22 16:12           ` zimoun
@ 2021-01-11 12:29             ` zimoun
  0 siblings, 0 replies; 23+ messages in thread
From: zimoun @ 2021-01-11 12:29 UTC (permalink / raw)
  To: Danny Milosavljevic; +Cc: steve.sprang, 30087-done

Hi,

On Tue, 22 Dec 2020 at 17:12, zimoun <zimon.toutoune@gmail.com> wrote:

>> Is it still relevant?  If yes, could you provide more information to
>> tackle it?  For example, more context or recipe to reproduce it.  If no,
>> feel free to close it.
>
> If no news in the coming days about the relevance of this bug, I will
> took the liberty to close it.

So I am closing.  Feel free to reopen it if I have misunderstood
something.


All the best,
simon




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

* [bug#30053] [PATCH 1/3] Improve appearance of tabular output.
  2018-01-16 14:16         ` Ludovic Courtès
  2018-01-16 23:56           ` Steve Sprang
@ 2021-07-15  5:39           ` Maxim Cournoyer
  2021-07-15 17:36             ` [bug#30053] [PATCH 1/3 v2] " Maxim Cournoyer
  2021-07-15 22:05             ` Sarah Morgensen via Guix-patches via
  1 sibling, 2 replies; 23+ messages in thread
From: Maxim Cournoyer @ 2021-07-15  5:39 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 30053, Steve Sprang, Roel Janssen

[-- Attachment #1: Type: text/plain, Size: 3647 bytes --]

Hello!

ludo@gnu.org (Ludovic Courtès) writes:

> Steve Sprang <steve.sprang@gmail.com> skribis:
>
>> On Fri, Jan 12, 2018 at 5:28 AM, Roel Janssen <roel@gnu.org> wrote:
>>> If we use GNU awk instead of cut, I think any whitespace will work:
>>>   $ guix package -A | awk '{ print $1 "@" $2 }'
>>>
>>> And then we can optimize the output reading experience for our users
>>> instead of for the 'cut' program.
>>
>> I like this proposal, unless there is a strong reason to prefer 'cut'?
>
> Again a matter of taste, but ‘cut’ looks to me both easier and simpler
> than awk (since it’s a full language).
>
> But anyway, as Danny write, if your patches retain tabs (in addition to
> spaces), presumably it’s OK even for those of us who prefer ‘cut’,
> right?
>
> Thanks,
> Ludo’.

I rebased this set of patch, and modified them slightly (attached).  One
thing that got my attention is the performance.  For short lists of
packages, it's invisible, but it takes noticeably longer for 'guix
package -A', for example.  I'm not sure where the time gets spent (see:
https://paste.debian.net/1204412/).

This is for guix package -A:

--8<---------------cut here---------------start------------->8---
%     cumulative   self             
time   seconds     seconds  procedure
 17.28     37.22      3.61  guix/memoization.scm:100:0
  9.52      2.25      1.99  set-procedure-property!
  4.23      1.14      0.89  ice-9/vlist.scm:539:0:vhash-assq
  3.70      0.77      0.77  ice-9/popen.scm:183:0:reap-pipes
  3.00      0.63      0.63  ice-9/eval.scm:604:6
  2.82      0.66      0.59  open-output-string
  2.65      1.36      0.55  srfi/srfi-1.scm:1028:0:lset-intersection
  2.29      0.48      0.48  write-char
  2.12      0.44      0.44  display
  1.94      0.44      0.41  ice-9/boot-9.scm:2217:0:%load-announce
  1.59      0.33      0.33  hash-ref
  1.59      0.33      0.33  hashq
  1.41      0.41      0.30  ice-9/vlist.scm:449:0:vhash-cons
  1.23      3.58      0.26  ice-9/format.scm:113:2:format:format-work
  1.23      2.99      0.26  ice-9/format.scm:39:0:format
  1.23      0.33      0.26  srfi/srfi-1.scm:1033:17
  1.06      0.30      0.22  guix/packages.scm:924:6:mproc
  1.06      0.22      0.22  string=?
  1.06      0.22      0.22  hash-set!
  1.06      0.22      0.22  procedure?
  1.06      0.22      0.22  ice-9/boot-9.scm:3569:0:autoload-done-or-in-progress?
  1.06      0.22      0.22  append
  1.06      0.22      0.22  reverse!
  0.88      2.80      0.18  guix/build-system/cargo.scm:246:0:lower
  0.88      0.37      0.18  ice-9/eval.scm:297:11
  0.88      0.18      0.18  list?
  0.71     43.08      0.15  ice-9/eval.scm:292:11
  0.71     24.34      0.15  guix/packages.scm:926:16
  0.71      0.18      0.15  make-string
  0.53    246.72      0.11  ice-9/threads.scm:388:4
  0.53     32.97      0.11  guix/packages.scm:924:6
  0.53      2.07      0.11  ice-9/eval.scm:159:9
  0.53      1.33      0.11  ice-9/format.scm:759:2:format:out-obj-padded
  0.53      0.15      0.11  get-output-string
  0.53      0.11      0.11  ice-9/eval.scm:126:12
  0.53      0.11      0.11  reverse
  [...]
---
Sample count: 567
Total time: 20.913633405 seconds (12.747006885 seconds in GC)
--8<---------------cut here---------------end--------------->8---

Without the change 'guix package -A' runs in about 2 seconds.  With the
change it runs in about 12 seconds here.

Danny's suggestion to use 'guix package -A | columns -t' works too, but
it's not convenient nor discoverable.

Any opinions?  Otherwise I might throw a coin, as I'm 50/50 on this.


[-- Attachment #2: 0001-utils-Add-a-procedure-for-pretty-printing-tabular-da.patch --]
[-- Type: text/x-patch, Size: 2525 bytes --]

From faf27c47211281628dc5e216b316583da503dcd1 Mon Sep 17 00:00:00 2001
From: Steve Sprang <scs@stevesprang.com>
Date: Tue, 9 Jan 2018 14:00:11 -0800
Subject: [PATCH 1/4] utils: Add a procedure for pretty printing tabular data.

* guix/utils.scm (pretty-print-table): New procedure.

Co-authored-by: Maxim Cournoyer <maxim.cournoyer@gmail.com>
Signed-off-by: Maxim Cournoyer <maxim.cournoyer@gmail.com>
---
 guix/utils.scm | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/guix/utils.scm b/guix/utils.scm
index 05af86fc37..d43ff8d719 100644
--- a/guix/utils.scm
+++ b/guix/utils.scm
@@ -10,6 +10,8 @@
 ;;; Copyright © 2020 Efraim Flashner <efraim@flashner.co.il>
 ;;; Copyright © 2021 Simon Tournier <zimon.toutoune@gmail.com>
 ;;; Copyright © 2021 Chris Marusich <cmmarusich@gmail.com>
+;;; Copyright © 2018 Steve Sprang <scs@stevesprang.com>
+;;; Copyright © 2021 Maxim Cournoyer <maxim.cournoyer@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -123,7 +125,9 @@
             canonical-newline-port
 
             string-distance
-            string-closest))
+            string-closest
+
+            pretty-print-table))
 
 \f
 ;;;
@@ -935,6 +939,27 @@ according to THRESHOLD, then #f is returned."
            #f +inf.0
            tests)))
 
+\f
+;;;
+;;; Prettified output.
+;;;
+
+(define (pretty-print-table rows)
+  "Print ROWS in neat columns.  All rows should be lists of strings and each
+row should have the same length.  The columns are separated by a tab
+character, and aligned using spaces."
+  (let* ((number-of-columns-to-pad (if (null? rows) 0 (1- (length (first rows)))))
+         ;; Ignore the last column as it is left aligned and doesn't need
+         ;; padding; this prevents printing extraneous trailing spaces.
+         (column-widths (fold (lambda (row maximums)
+                                (map max (map string-length row) maximums))
+                              ;; Initial max width is 0 for each column.
+                              (make-list number-of-columns-to-pad 0)
+                              (map (cut drop-right <> 1) rows)))
+         (column-formats (map (cut format #f "~~~da" <>) column-widths))
+         (fmt (string-append (string-join column-formats "\t") "\t~a")))
+    (map (cut format #t "~?~%" fmt <>) rows)))
+
 ;;; Local Variables:
 ;;; eval: (put 'call-with-progress-reporter 'scheme-indent-function 1)
 ;;; End:
-- 
2.32.0


[-- Attachment #3: 0002-package-Improve-output-appearance-when-listing-packa.patch --]
[-- Type: text/x-patch, Size: 3479 bytes --]

From 81d7ada26c30effede23926fb85b2b8ed59778af Mon Sep 17 00:00:00 2001
From: Steve Sprang <scs@stevesprang.com>
Date: Tue, 9 Jan 2018 14:10:04 -0800
Subject: [PATCH 2/4] package: Improve output appearance when listing packages.

* guix/scripts/package.scm (process-query): Use pretty-print-table when
listing installed and available packages.

Modified-by: Maxim Cournoyer <maxim.cournoyer@gmail.com>
Signed-off-by: Maxim Cournoyer <maxim.cournoyer@gmail.com>
---
 guix/scripts/package.scm | 37 ++++++++++++++++++-------------------
 1 file changed, 18 insertions(+), 19 deletions(-)

diff --git a/guix/scripts/package.scm b/guix/scripts/package.scm
index 694959d326..a34ecdcb54 100644
--- a/guix/scripts/package.scm
+++ b/guix/scripts/package.scm
@@ -9,6 +9,7 @@
 ;;; Copyright © 2019 Tobias Geerinckx-Rice <me@tobias.gr>
 ;;; Copyright © 2020 Ricardo Wurmus <rekado@elephly.net>
 ;;; Copyright © 2020 Simon Tournier <zimon.toutoune@gmail.com>
+;;; Copyright © 2018 Steve Sprang <scs@stevesprang.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -831,15 +832,14 @@ processed, #f otherwise."
                           (map profile-manifest profiles)))
               (installed (manifest-entries manifest)))
          (leave-on-EPIPE
-          (for-each (match-lambda
-                      (($ <manifest-entry> name version output path _)
-                       (when (or (not regexp)
-                                 (regexp-exec regexp name))
-                         (format #t "~a\t~a\t~a\t~a~%"
-                                 name (or version "?") output path))))
-
-                    ;; Show most recently installed packages last.
-                    (reverse installed))))
+          (let ((rows (filter-map
+                       (match-lambda
+                         (($ <manifest-entry> name version output path _)
+                          (and (regexp-exec regexp name)
+                               (list name (or version "?") output path))))
+                       installed)))
+            ;; Show most recently installed packages last.
+            (pretty-print-table (reverse rows)))))
        #t)
 
       (('list-available regexp)
@@ -862,16 +862,15 @@ processed, #f otherwise."
                                 result))
                           '())))
          (leave-on-EPIPE
-          (for-each (match-lambda
-                      ((name version outputs location)
-                       (format #t "~a\t~a\t~a\t~a~%"
-                               name version
-                               (string-join outputs ",")
-                               (location->string location))))
-                    (sort available
-                          (match-lambda*
-                            (((name1 . _) (name2 . _))
-                             (string<? name1 name2))))))
+          (let ((rows (map (match-lambda
+                             ((name version outputs location)
+                              (list name version (string-join outputs ",")
+                                    (location->string location))))
+                           (sort available
+                                 (match-lambda*
+                                   (((name1 . _) (name2 . _))
+                                    (string<? name1 name2)))))))
+            (pretty-print-table rows)))
          #t))
 
       (('list-profiles _)
-- 
2.32.0


[-- Attachment #4: 0003-ui-Improve-output-appearance-when-listing-generation.patch --]
[-- Type: text/x-patch, Size: 3276 bytes --]

From 43a99413e4a59ed32887b8b0552353637f2b7304 Mon Sep 17 00:00:00 2001
From: Steve Sprang <scs@stevesprang.com>
Date: Tue, 9 Jan 2018 14:20:12 -0800
Subject: [PATCH 3/4] ui: Improve output appearance when listing generations.

* guix/ui.scm (display-profile-content-diff): Use pretty-print-table to format
output.
(display-profile-content): Likewise.
---
 guix/ui.scm | 29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/guix/ui.scm b/guix/ui.scm
index 26a437e904..1428c254b3 100644
--- a/guix/ui.scm
+++ b/guix/ui.scm
@@ -16,6 +16,7 @@
 ;;; Copyright © 2019, 2021 Simon Tournier <zimon.toutoune@gmail.com>
 ;;; Copyright © 2020 Arun Isaac <arunisaac@systemreboot.net>
 ;;; Copyright © 2020 Maxim Cournoyer <maxim.cournoyer@gmail.com>
+;;; Copyright © 2018 Steve Sprang <scs@stevesprang.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -1889,10 +1890,10 @@ DURATION-RELATION with the current time."
   (define (equal-entry? first second)
     (string= (manifest-entry-item first) (manifest-entry-item second)))
 
-  (define (display-entry entry prefix)
+  (define (make-row entry prefix)
     (match entry
       (($ <manifest-entry> name version output location _)
-       (format #t " ~a ~a\t~a\t~a\t~a~%" prefix name version output location))))
+       (list (format #f " ~a ~a" prefix name) version output location))))
 
   (define (list-entries number)
     (manifest-entries (profile-manifest (generation-file-name profile number))))
@@ -1903,8 +1904,8 @@ DURATION-RELATION with the current time."
                   equal-entry? (list-entries new) (list-entries old)))
           (removed (lset-difference
                     equal-entry? (list-entries old) (list-entries new))))
-      (for-each (cut display-entry <> "+") added)
-      (for-each (cut display-entry <> "-") removed)
+      (pretty-print-table (append (map (cut make-row <> "+") added)
+                                  (map (cut make-row <> "-") removed)))
       (newline)))
 
   (display-diff profile gen1 gen2))
@@ -1932,15 +1933,17 @@ already taken."
 (define (display-profile-content profile number)
   "Display the packages in PROFILE, generation NUMBER, in a human-readable
 way."
-  (for-each (match-lambda
-              (($ <manifest-entry> name version output location _)
-               (format #t "  ~a\t~a\t~a\t~a~%"
-                       name version output location)))
-
-            ;; Show most recently installed packages last.
-            (reverse
-             (manifest-entries
-              (profile-manifest (generation-file-name profile number))))))
+
+  (define entry->row
+    (match-lambda
+      (($ <manifest-entry> name version output location _)
+       (list (string-append "  " name) version output location))))
+
+  (let* ((manifest (profile-manifest (generation-file-name profile number)))
+         (entries  (manifest-entries manifest))
+         (rows     (map entry->row entries)))
+    ;; Show most recently installed packages last.
+    (pretty-print-table (reverse rows))))
 
 (define (display-generation-change previous current)
   (format #t (G_ "switched from generation ~a to ~a~%") previous current))
-- 
2.32.0


[-- Attachment #5: Type: text/plain, Size: 16 bytes --]


Thanks,

Maxim

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

* [bug#30053] [PATCH 1/3 v2] Improve appearance of tabular output.
  2021-07-15  5:39           ` Maxim Cournoyer
@ 2021-07-15 17:36             ` Maxim Cournoyer
  2021-07-15 20:15               ` bug#30053: [PATCH 1/3] " Maxim Cournoyer
  2021-07-21 16:56               ` [bug#30053] " Ludovic Courtès
  2021-07-15 22:05             ` Sarah Morgensen via Guix-patches via
  1 sibling, 2 replies; 23+ messages in thread
From: Maxim Cournoyer @ 2021-07-15 17:36 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 30053, Steve Sprang, Roel Janssen

[-- Attachment #1: Type: text/plain, Size: 367 bytes --]

Hi,

Here's an improved version of the first patch.  It now uses a hard limit
on the maximum width of a column, which is a poor man's way of getting
rid of the outliers that cause the table to be too wide in some
situations (such as in 'guix package -A').

Otherwise the performance remain unchanged (from ~2 to ~7 seconds with
'guix package -A' on a fast machine).


[-- Attachment #2: 0001-utils-Add-a-procedure-for-pretty-printing-tabular-da.patch --]
[-- Type: text/x-patch, Size: 2872 bytes --]

From d8fd6c9a1b8677cd69e50fe4f3e50c60c5fb7e35 Mon Sep 17 00:00:00 2001
From: Steve Sprang <scs@stevesprang.com>
Date: Tue, 9 Jan 2018 14:00:11 -0800
Subject: [PATCH] utils: Add a procedure for pretty printing tabular data.

* guix/utils.scm (pretty-print-table): New procedure.

Co-authored-by: Maxim Cournoyer <maxim.cournoyer@gmail.com>
Signed-off-by: Maxim Cournoyer <maxim.cournoyer@gmail.com>
---
 guix/utils.scm | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/guix/utils.scm b/guix/utils.scm
index 05af86fc37..f2506d38b4 100644
--- a/guix/utils.scm
+++ b/guix/utils.scm
@@ -10,6 +10,8 @@
 ;;; Copyright © 2020 Efraim Flashner <efraim@flashner.co.il>
 ;;; Copyright © 2021 Simon Tournier <zimon.toutoune@gmail.com>
 ;;; Copyright © 2021 Chris Marusich <cmmarusich@gmail.com>
+;;; Copyright © 2018 Steve Sprang <scs@stevesprang.com>
+;;; Copyright © 2021 Maxim Cournoyer <maxim.cournoyer@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -123,7 +125,9 @@
             canonical-newline-port
 
             string-distance
-            string-closest))
+            string-closest
+
+            pretty-print-table))
 
 \f
 ;;;
@@ -935,6 +939,33 @@ according to THRESHOLD, then #f is returned."
            #f +inf.0
            tests)))
 
+\f
+;;;
+;;; Prettified output.
+;;;
+
+(define* (pretty-print-table rows #:key (max-column-width 20))
+  "Print ROWS in neat columns.  All rows should be lists of strings and each
+row should have the same length.  The columns are separated by a tab
+character, and aligned using spaces.  The maximum width of each column is
+bound by MAX-COLUMN-WIDTH."
+  (let* ((number-of-columns-to-pad (if (null? rows)
+                                       0
+                                       (1- (length (first rows)))))
+         ;; Ignore the last column as it is left aligned and doesn't need
+         ;; padding; this prevents printing extraneous trailing spaces.
+         (column-widths (fold (lambda (row maximums)
+                                (map (cut min <> max-column-width)
+                                     (map max
+                                          (map string-length row)
+                                          maximums)))
+                              ;; Initial max width is 0 for each column.
+                              (make-list number-of-columns-to-pad 0)
+                              (map (cut drop-right <> 1) rows)))
+         (column-formats (map (cut format #f "~~~da" <>) column-widths))
+         (fmt (string-append (string-join column-formats "\t") "\t~a")))
+    (for-each (cut format #t "~?~%" fmt <>) rows)))
+
 ;;; Local Variables:
 ;;; eval: (put 'call-with-progress-reporter 'scheme-indent-function 1)
 ;;; End:
-- 
2.32.0


[-- Attachment #3: Type: text/plain, Size: 16 bytes --]


Thanks!

Maxim

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

* bug#30053: [PATCH 1/3] Improve appearance of tabular output.
  2021-07-15 17:36             ` [bug#30053] [PATCH 1/3 v2] " Maxim Cournoyer
@ 2021-07-15 20:15               ` Maxim Cournoyer
  2021-07-21 16:56               ` [bug#30053] " Ludovic Courtès
  1 sibling, 0 replies; 23+ messages in thread
From: Maxim Cournoyer @ 2021-07-15 20:15 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 30053-done, Steve Sprang, Roel Janssen

Hi again,

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

> Hi,
>
> Here's an improved version of the first patch.  It now uses a hard limit
> on the maximum width of a column, which is a poor man's way of getting
> rid of the outliers that cause the table to be too wide in some
> situations (such as in 'guix package -A').
>
> Otherwise the performance remain unchanged (from ~2 to ~7 seconds with
> 'guix package -A' on a fast machine).
>
>
>
> Thanks!
>
> Maxim

I've improved it some more and pushed as 01d7e8c278.  The performance is
about 5 s on a fast machines to format the 18000 something packages of
the collection, compared to 2 s before (guix package -A), which I think
is OK (but it'd be nice to see Guile opitimize for faster (ice-9
format)!).

Closing.

Thanks to all involved!

Maxim




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

* [bug#30053] [PATCH 1/3] Improve appearance of tabular output.
  2021-07-15  5:39           ` Maxim Cournoyer
  2021-07-15 17:36             ` [bug#30053] [PATCH 1/3 v2] " Maxim Cournoyer
@ 2021-07-15 22:05             ` Sarah Morgensen via Guix-patches via
  2021-07-16  1:25               ` Maxim Cournoyer
  1 sibling, 1 reply; 23+ messages in thread
From: Sarah Morgensen via Guix-patches via @ 2021-07-15 22:05 UTC (permalink / raw)
  To: Maxim Cournoyer; +Cc: 30053, Ludovic Courtès, Steve Sprang, Roel Janssen

Hi Maxim,

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

> [...]
> ---
> Sample count: 567
> Total time: 20.913633405 seconds (12.747006885 seconds in GC)
>
> Without the change 'guix package -A' runs in about 2 seconds.  With the
> change it runs in about 12 seconds here.

I cannot replicate this. Without the patch on master (7e0da2f):

--8<---------------cut here---------------start------------->8---
$ time ./pre-inst-env guix package -A > /dev/null

real	0m5.473s
user	0m6.698s
sys	0m0.094s
--8<---------------cut here---------------end--------------->8---

And with the patch:

--8<---------------cut here---------------start------------->8---
$ time ./pre-inst-env guix package -A > /dev/null

real	0m5.778s
user	0m6.862s
sys	0m0.061s
--8<---------------cut here---------------end--------------->8---

Perhaps there's something else going on there? I'm on x86-64, if that's
useful.

> Danny's suggestion to use 'guix package -A | columns -t' works too, but
> it's not convenient nor discoverable.

Definitely agree, though in my opinion neither that nor this *really*
make `guix package --list-installed` pretty. I'm sure I could put
together an alias but it goes a long way toward making Guix look
polished to have it built-in.

>
> Any opinions?  Otherwise I might throw a coin, as I'm 50/50 on this.
>
>
>
>
>
> Thanks,
>
> Maxim

--
Sarah




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

* [bug#30053] [PATCH 1/3] Improve appearance of tabular output.
  2021-07-15 22:05             ` Sarah Morgensen via Guix-patches via
@ 2021-07-16  1:25               ` Maxim Cournoyer
  0 siblings, 0 replies; 23+ messages in thread
From: Maxim Cournoyer @ 2021-07-16  1:25 UTC (permalink / raw)
  To: Sarah Morgensen; +Cc: 30053, Ludovic Courtès, Steve Sprang, Roel Janssen

Hello,

Sarah Morgensen <iskarian@mgsn.dev> writes:

> Hi Maxim,
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
>
>> [...]
>> ---
>> Sample count: 567
>> Total time: 20.913633405 seconds (12.747006885 seconds in GC)
>>
>> Without the change 'guix package -A' runs in about 2 seconds.  With the
>> change it runs in about 12 seconds here.
>
> I cannot replicate this. Without the patch on master (7e0da2f):
>
> $ time ./pre-inst-env guix package -A > /dev/null
>
> real	0m5.473s
> user	0m6.698s
> sys	0m0.094s
>
>
> And with the patch:
>
> $ time ./pre-inst-env guix package -A > /dev/null
>
> real	0m5.778s
> user	0m6.862s
> sys	0m0.061s

I tested on a different machine after tweakwing the code slightly today,
and the results were not as bad as those I reported earlier.  Buffering
the output helped a lot.

> Perhaps there's something else going on there? I'm on x86-64, if that's
> useful.
>
>> Danny's suggestion to use 'guix package -A | columns -t' works too, but
>> it's not convenient nor discoverable.
>
> Definitely agree, though in my opinion neither that nor this *really*
> make `guix package --list-installed` pretty. I'm sure I could put
> together an alias but it goes a long way toward making Guix look
> polished to have it built-in.

Thanks for sharing your opinion!  I ended up pushing it to the repo a
bit earlier today; after I found it was running acceptably fast and set
an upper limit to the maximum column width so that the output would
remain compact enough.

You should have it available to try on your next 'guix pull', if you
haven't already :-).

Thanks,

Maxim




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

* [bug#30053] [PATCH 1/3] Improve appearance of tabular output.
  2021-07-15 17:36             ` [bug#30053] [PATCH 1/3 v2] " Maxim Cournoyer
  2021-07-15 20:15               ` bug#30053: [PATCH 1/3] " Maxim Cournoyer
@ 2021-07-21 16:56               ` Ludovic Courtès
  2021-07-21 21:43                 ` Maxim Cournoyer
  1 sibling, 1 reply; 23+ messages in thread
From: Ludovic Courtès @ 2021-07-21 16:56 UTC (permalink / raw)
  To: Maxim Cournoyer; +Cc: 30053, Steve Sprang, Roel Janssen

Hi Maxim and all,

Thank you for unlocking this old patch series.  :-)

Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

>>From d8fd6c9a1b8677cd69e50fe4f3e50c60c5fb7e35 Mon Sep 17 00:00:00 2001
> From: Steve Sprang <scs@stevesprang.com>
> Date: Tue, 9 Jan 2018 14:00:11 -0800
> Subject: [PATCH] utils: Add a procedure for pretty printing tabular data.
>
> * guix/utils.scm (pretty-print-table): New procedure.
>
> Co-authored-by: Maxim Cournoyer <maxim.cournoyer@gmail.com>
> Signed-off-by: Maxim Cournoyer <maxim.cournoyer@gmail.com>

[...]

> +(define* (pretty-print-table rows #:key (max-column-width 20))
> +  "Print ROWS in neat columns.  All rows should be lists of strings and each
> +row should have the same length.  The columns are separated by a tab
> +character, and aligned using spaces.  The maximum width of each column is
> +bound by MAX-COLUMN-WIDTH."

The version that was pushed has:

  (setvbuf (current-output-port) 'block)

I’m in favor of removing it because it’s “impolite” so to speak :-) to
have such a side effect buried here.  (guix ui) enables line-buffering
on startup anyway.

Ludo’.

PS: Commit 01d7e8c2782f61e741f8beff7888adfbdb61779d shows an
    incompatibility with some previously-fine uses of ‘cut’, but surely
    that was the price to pay.  (An option would be to behave
    differently depending on whether stdout is a tty or not, but that’s
    probably bad style…)




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

* [bug#30053] [PATCH 1/3] Improve appearance of tabular output.
  2021-07-21 16:56               ` [bug#30053] " Ludovic Courtès
@ 2021-07-21 21:43                 ` Maxim Cournoyer
  0 siblings, 0 replies; 23+ messages in thread
From: Maxim Cournoyer @ 2021-07-21 21:43 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 30053, Steve Sprang, Roel Janssen

Hello,

Ludovic Courtès <ludo@gnu.org> writes:

> Hi Maxim and all,
>
> Thank you for unlocking this old patch series.  :-)
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
>>>From d8fd6c9a1b8677cd69e50fe4f3e50c60c5fb7e35 Mon Sep 17 00:00:00 2001
>> From: Steve Sprang <scs@stevesprang.com>
>> Date: Tue, 9 Jan 2018 14:00:11 -0800
>> Subject: [PATCH] utils: Add a procedure for pretty printing tabular data.
>>
>> * guix/utils.scm (pretty-print-table): New procedure.
>>
>> Co-authored-by: Maxim Cournoyer <maxim.cournoyer@gmail.com>
>> Signed-off-by: Maxim Cournoyer <maxim.cournoyer@gmail.com>
>
> [...]
>
>> +(define* (pretty-print-table rows #:key (max-column-width 20))
>> +  "Print ROWS in neat columns.  All rows should be lists of strings and each
>> +row should have the same length.  The columns are separated by a tab
>> +character, and aligned using spaces.  The maximum width of each column is
>> +bound by MAX-COLUMN-WIDTH."
>
> The version that was pushed has:
>
>   (setvbuf (current-output-port) 'block)
>
> I’m in favor of removing it because it’s “impolite” so to speak :-) to
> have such a side effect buried here.  (guix ui) enables line-buffering
> on startup anyway.

Hehe, apologies.  In my experiments, using 'block buffering seemed to
improve performance a lot.  Testing it again, the difference is
insignificant.  So I'm happy to attribute this to a measurement error on
my part (perhaps I had forgotten to recompile the modified module,
leading to the discrepancy, or perhaps another process was loading the
system).

Pushed with as commit 4f51a4ac27.

> Ludo’.
>
> PS: Commit 01d7e8c2782f61e741f8beff7888adfbdb61779d shows an
>     incompatibility with some previously-fine uses of ‘cut’, but surely
>     that was the price to pay.  (An option would be to behave
>     differently depending on whether stdout is a tty or not, but that’s
>     probably bad style…)

I pondered about that (using isatty), but considering people might pipe
the output to less to interactively view it, that doesn't seem to be a
good idea.

Thanks for the feedback!

Maxim




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

end of thread, other threads:[~2021-07-21 21:44 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-09 22:34 [bug#30053] [PATCH 1/3] Improve appearance of tabular output Steve Sprang
2018-01-09 22:37 ` [bug#30053] [PATCH 2/3] " Steve Sprang
2018-01-09 22:37 ` [bug#30053] [PATCH 3/3] " Steve Sprang
2018-01-11 21:32 ` [bug#30053] [PATCH 1/3] " Ludovic Courtès
2018-01-11 23:32   ` Steve Sprang
2018-01-12 13:28     ` Roel Janssen
2018-01-13 19:59       ` Steve Sprang
2018-01-16 14:16         ` Ludovic Courtès
2018-01-16 23:56           ` Steve Sprang
2021-07-15  5:39           ` Maxim Cournoyer
2021-07-15 17:36             ` [bug#30053] [PATCH 1/3 v2] " Maxim Cournoyer
2021-07-15 20:15               ` bug#30053: [PATCH 1/3] " Maxim Cournoyer
2021-07-21 16:56               ` [bug#30053] " Ludovic Courtès
2021-07-21 21:43                 ` Maxim Cournoyer
2021-07-15 22:05             ` Sarah Morgensen via Guix-patches via
2021-07-16  1:25               ` Maxim Cournoyer
2018-01-12 14:56   ` Danny Milosavljevic
2018-01-12 15:26     ` Leo Famulari
2018-01-12 15:50       ` bug#30087: "guix package -A" hangs with attached package set Danny Milosavljevic
2020-12-03  0:10         ` zimoun
2020-12-22 16:12           ` zimoun
2021-01-11 12:29             ` zimoun
2018-01-13 13:47       ` [bug#30053] [PATCH 1/3] Improve appearance of tabular output Ludovic Courtès

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/guix.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.