[PATCH 1/3] Improve appearance of tabular output.

  • Done
  • quality assurance status badge
Details
7 participants
  • Danny Milosavljevic
  • Sarah Morgensen
  • Leo Famulari
  • Ludovic Courtès
  • Maxim Cournoyer
  • Roel Janssen
  • Steve Sprang
Owner
unassigned
Submitted by
Steve Sprang
Severity
normal
S
S
Steve Sprang wrote on 9 Jan 2018 23:34
(address . guix-patches@gnu.org)
CA+xn8YBKpjVYmrhKbR7Y=wZnnfr+ALuEMHo_b5AEFMAOTEkDxA@mail.gmail.com
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
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(-)

Toggle diff (42 lines)
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))))))
+;;;
+;;; 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)))
+
+
;;;
;;; Keyword arguments.
;;;
--
2.15.1
S
S
Steve Sprang wrote on 9 Jan 2018 23:37
[PATCH 2/3] Improve appearance of tabular output.
(address . 30053@debbugs.gnu.org)
CA+xn8YBAvzHe57HEGDcFGKTVeXcx=nA7Rsg31ghWLL6_ujJufg@mail.gmail.com

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

Toggle diff (57 lines)
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
S
S
Steve Sprang wrote on 9 Jan 2018 23:37
[PATCH 3/3] Improve appearance of tabular output.
(address . 30053@debbugs.gnu.org)
CA+xn8YBnEa0qCs_FKn-gCnz0g+UJwy5LziMcDSyzODNMPF_qbQ@mail.gmail.com

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

Toggle diff (57 lines)
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
L
L
Ludovic Courtès wrote on 11 Jan 2018 22:32
Re: [bug#30053] [PATCH 1/3] Improve appearance of tabular output.
(name . Steve Sprang)(address . steve.sprang@gmail.com)(address . 30053@debbugs.gnu.org)
878td4nksy.fsf@gnu.org
Hello Steve,

Long time no see! ;-)

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

Toggle quote (8 lines)
> 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’:

Toggle snippet (13 lines)
$ guix package -A | cut -f1 | head
0ad
0ad-data
0xffff
4store
4ti2
a2ps
aalib
abbaye
abc
abcde

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’.
S
S
Steve Sprang wrote on 12 Jan 2018 00:32
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 30053@debbugs.gnu.org)
CA+xn8YDjApPwzx_1kR2pTuwVAyajXO+n5KN27WCGNAF2eX+rDg@mail.gmail.com
Hi Ludovic,

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

Yeah, it's been a while!

Toggle quote (18 lines)
> 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.

Toggle quote (5 lines)
> 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.

Toggle quote (6 lines)
> 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
R
R
Roel Janssen wrote on 12 Jan 2018 14:28
(name . Steve Sprang)(address . steve.sprang@gmail.com)
87fu7b2old.fsf@gnu.org
Steve Sprang writes:

Toggle quote (49 lines)
> 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
D
D
Danny Milosavljevic wrote on 12 Jan 2018 15:56
(name . Ludovic Courtès)(address . ludo@gnu.org)
20180112155612.5998bc36@scratchpost.org
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").

Toggle quote (4 lines)
> 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
L
L
Leo Famulari wrote on 12 Jan 2018 16:26
(name . Danny Milosavljevic)(address . dannym@scratchpost.org)
20180112152631.GB15918@jasmine.lan
On Fri, Jan 12, 2018 at 03:56:12PM +0100, Danny Milosavljevic wrote:
Toggle quote (4 lines)
> 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?
-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEEsFFZSPHn08G5gDigJkb6MLrKfwgFAlpY06cACgkQJkb6MLrK
fwizbBAA4lJC8i7JTBgoxcsE9+79UETN/NpsbU1sQt3ADuYlrY2v1dT9N+r6QtFD
7PRu/voVdTOlVoXRfEs/SDjaCVQReeRb793nBWGpoPGEd560FKL97I1mgWD+6CeB
mCvMgr/vyMIwvMVeIXnJtVsF2hGJyYpYc8hmo5vMcfiUMQNi9YDb/J9vZ0zUoGPj
fj/MHzqQ1WooLrNN+l5bGFPfm5ZaPJQpjjB1peMrqsTfS9YF/iaobiuor1KZKxVj
Ku9rtwvdAEMkG4CIzq2Xd4wu65MDAKxUBLTXBfOmLcbTb9VQP7f1WiPmAYhRuyr/
1MvrwDOoSSYYYymBL6efgB3DvYVRQu2LREPFEaYNjnYeH8frfInZF1zibU0bN0Ac
J8LAasBDBo2IjYmp06qUhKp0ijyuhNEIQdNXs2X1avrRa05rwMXMqoAawaBRBIkU
uSijk5AJxStUz1F5U1tI0oTZENiKXJnM5Qi1YwrzbHAHgRezJ1gjXeKGaOh8jyVj
W75zBxNAiAdCLxfnCY88qHLZorS8CsuIYxOrSro+BGPhWeo021koKafF23g0TNl3
ZbRDk7ai+2fiB800v3lUXH612Ul2yCgJAFf5Hg5lVcLqQCUGHppUrGBrSUHNGmUX
erpnIFkUyKGW2fbOWlij/jv0fzi/3VMTTw4ZqciQsKwDb2OanoY=
=mwL/
-----END PGP SIGNATURE-----


L
L
Ludovic Courtès wrote on 13 Jan 2018 14:47
(name . Leo Famulari)(address . leo@famulari.name)
87y3l1q39y.fsf@gnu.org
Leo Famulari <leo@famulari.name> skribis:

Toggle quote (8 lines)
> 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’.
S
S
Steve Sprang wrote on 13 Jan 2018 20:59
(name . Roel Janssen)(address . roel@gnu.org)
CA+xn8YB=fnhMS32j-UdGRJy0ruH=rQvaNGycehtLewji_nV6zg@mail.gmail.com
On Fri, Jan 12, 2018 at 5:28 AM, Roel Janssen <roel@gnu.org> wrote:
Toggle quote (6 lines)
> 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
L
L
Ludovic Courtès wrote on 16 Jan 2018 15:16
(name . Steve Sprang)(address . steve.sprang@gmail.com)
87zi5d6g8f.fsf@gnu.org
Steve Sprang <steve.sprang@gmail.com> skribis:

Toggle quote (9 lines)
> 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’.
S
S
Steve Sprang wrote on 17 Jan 2018 00:56
(name . Ludovic Courtès)(address . ludo@gnu.org)
CA+xn8YBTbud0kkTLSbciyKKF86m8Uoe_AGV-NSSH8U=QD24ifA@mail.gmail.com
On Tue, Jan 16, 2018 at 6:16 AM, Ludovic Courtès <ludo@gnu.org> wrote:

Toggle quote (7 lines)
> 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
M
M
Maxim Cournoyer wrote on 15 Jul 2021 07:39
Re: bug#30053: [PATCH 1/3] Improve appearance of tabular output.
(name . Ludovic Courtès)(address . ludo@gnu.org)
87im1cnnjb.fsf_-_@gmail.com
Hello!

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

Toggle quote (21 lines)
> 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:

This is for guix package -A:

Toggle snippet (44 lines)
% 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)

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

Toggle diff (54 lines)
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))
;;;
@@ -935,6 +939,27 @@ according to THRESHOLD, then #f is returned."
#f +inf.0
tests)))
+
+;;;
+;;; 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
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(-)

Toggle diff (64 lines)
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
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(-)

Toggle diff (65 lines)
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
Thanks,

Maxim
M
M
Maxim Cournoyer wrote on 15 Jul 2021 19:36
Re: bug#30053: [PATCH 1/3 v2] Improve appearance of tabular output.
(name . Ludovic Courtès)(address . ludo@gnu.org)
871r7zcwdr.fsf_-_@gmail.com
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).
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(-)

Toggle diff (60 lines)
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))
;;;
@@ -935,6 +939,33 @@ according to THRESHOLD, then #f is returned."
#f +inf.0
tests)))
+
+;;;
+;;; 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
Thanks!

Maxim
M
M
Maxim Cournoyer wrote on 15 Jul 2021 22:15
Re: bug#30053: [PATCH 1/3] Improve appearance of tabular output.
(name . Ludovic Courtès)(address . ludo@gnu.org)
87wnprbagf.fsf_-_@gmail.com
Hi again,

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

Toggle quote (16 lines)
> 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
Closed
S
S
Sarah Morgensen wrote on 16 Jul 2021 00:05
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
861r7zjkrl.fsf_-_@mgsn.dev
Hi Maxim,

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

Toggle quote (8 lines)
> [...]
> ---
> 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):

Toggle snippet (7 lines)
$ time ./pre-inst-env guix package -A > /dev/null

real 0m5.473s
user 0m6.698s
sys 0m0.094s

And with the patch:

Toggle snippet (7 lines)
$ time ./pre-inst-env guix package -A > /dev/null

real 0m5.778s
user 0m6.862s
sys 0m0.061s

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

Toggle quote (3 lines)
> 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.

Toggle quote (11 lines)
>
> Any opinions? Otherwise I might throw a coin, as I'm 50/50 on this.
>
>
>
>
>
> Thanks,
>
> Maxim

--
Sarah
M
M
Maxim Cournoyer wrote on 16 Jul 2021 03:25
(name . Sarah Morgensen)(address . iskarian@mgsn.dev)
87eebznj6j.fsf@gmail.com
Hello,

Sarah Morgensen <iskarian@mgsn.dev> writes:

Toggle quote (29 lines)
> 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.

Toggle quote (11 lines)
> 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
L
L
Ludovic Courtès wrote on 21 Jul 2021 18:56
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
875yx3fvwh.fsf_-_@gnu.org
Hi Maxim and all,

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

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

Toggle quote (10 lines)
>>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>

[...]

Toggle quote (6 lines)
> +(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…)
M
M
Maxim Cournoyer wrote on 21 Jul 2021 23:43
(name . Ludovic Courtès)(address . ludo@gnu.org)
87fsw7jqbl.fsf@gmail.com
Hello,

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

Toggle quote (32 lines)
> 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.

Toggle quote (8 lines)
> 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
?