Thanks for the feedback! Here's an updated patch. Let me know if I missed
anything.

I agree that we can drop hours from the elapsed time (or adaptively add
them if needed). I will send a fix for that in a subsequent patch.

On Tue, Sep 15, 2015 at 1:22 AM, Ludovic Courtès <ludo@gnu.org> wrote:

> Steve Sprang <steve.sprang@gmail.com> skribis:
>
> > I added newlines to split up output and visually separate multiple
> > substitutions. Since the download size is unknown, we can't use a
> progress
> > bar, but I made the output more consistent with the progress bar
> behavior.
> > I also shortened the hash after it's been repeated in full twice (once
> for
> > the store and once for the download URL).
>
> Nice.
>
> > Patched behavior:
> >
> > Found valid signature for
> > /gnu/store/sin6c3n1440f8kza0k4hdms38fbb4dv0-boost-1.58.0
> > From
> http://hydra.gnu.org/nar/sin6c3n1440f8kza0k4hdms38fbb4dv0-boost-1.58.0
> > Downloading sin6c3…boost-1.58.0 (110.6MiB installed)...
> >  sin6c3…boost-1.58.0                      911KiB/s 00:00:11 | 9.4MiB
> > transferred
>
> Looks good to me.
>
> Regarding elapsed time, I wonder if we should get rid of hours–i.e.,
> print “00:11” in this case.  Most of the time downloads last less than
> an hour, and when it lasts more, it’s OK to either print 100:11 or
> 01:03:11.  WDYT?
>
> > From 95936bf25394d2985f9331cb8fa08d5b30cb64a5 Mon Sep 17 00:00:00 2001
> > From: Steve Sprang <scs@stevesprang.com>
> > Date: Mon, 14 Sep 2015 22:31:11 -0700
> > Subject: [PATCH] substitute: Improve readability of substitute-related
> output.
> >
> > * guix/build/download.scm (flexible-space, truncated-url): New
> procedures.
> >   (progress-proc): Generate a better indeterminate progress string.
> >   (nearest-exact-integer, seconds->string, byte-count->string): Move
> to...
> > * guix/utils.scm: ...here.
> > * guix/store.scm (truncated-store-path): New procedure.
> > * guix/scripts/substitute.scm (assert-valid-narinfo): Add newlines to
> output.
> >   (process-substitution): Use byte-count->string and
> truncated-store-path.
>
> In general, it is best to make a separate patch that simply moves
> procedures from one file to another.
>
> However, most of the time, (guix build …) modules must not use (guix …)
> modules (info "(guix) G-Expressions").  This is the case here.
>
> So what I would suggest is to export those helper procedures from (guix
> build download) itself, as is done for ‘progress-proc’ et al.
>
> > +(define* (flexible-space left right #:optional (columns 80))
> > +  "Return a string of spaces which can be used to separate LEFT and
> RIGHT so
> > +that RIGHT is justified to a width of COLUMNS."
>
> What about this slightly more idiomatic procedure instead:
>
>   (define (string-pad-middle left right len)
>     ;; + docstring
>     (string-append left
>                    (string-pad right (max 0 (- len (string-length
> left))))))
>
> > +(define (truncated-url url)
> > +  "Return a friendlier version of URL for display."
> > +  (let ((store-path (string-append (%store-prefix) "/" (basename url))))
> > +    ;; take advantage of the implementation for store paths
> > +    (truncated-store-path store-path)))
>
> What about calling it ‘store-url-abbreviation’, to convey that it’s
> similar in spirit to ‘uri-abbreviation’, but specialized?
>
> >  (define* (progress-proc file size #:optional (log-port
> (current-output-port)))
> >    "Return a procedure to show the progress of FILE's download, which is
> SIZE
> >  bytes long.  The returned procedure is suitable for use as an argument
> to
> > @@ -130,24 +115,26 @@ bytes long.  The returned procedure is suitable
> for use as an argument to
> >                                           (seconds->string elapsed)
> >                                           (progress-bar %) %))
> >                       ;; TODO: Make this adapt to the actual terminal
> width.
> > -                     (cols       80)
> > -                     (num-spaces (max 1 (- cols (+ (string-length left)
> > -                                                   (string-length
> right)))))
> > -                     (gap        (make-string num-spaces #\space)))
> > +                     (gap        (flexible-space left right)))
> >                  (format log-port "~a~a~a" left gap right)
>
> This would become:
>
>   (display (string-pad-middle left right cols) log-port)
>
> Could you send an updated patch?
>
> Thank you!
>
> Ludo’.
>