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

[Maxim Cournoyer <maxim.cournoyer@gmail.com>]

[-- 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