From: Steve Sprang <steve.sprang@gmail.com>
To: Mathieu Lirzin <mthl@openmailbox.org>
Cc: guix-devel@gnu.org
Subject: Re: [PATCH] download: Fix some minor progress-logging regressions.
Date: Thu, 17 Sep 2015 13:52:55 -0700 [thread overview]
Message-ID: <CA+xn8YC788ttPLy-Fi4K20PnVwBr0Lg7GPSE+cjTAYqo5puoZA@mail.gmail.com> (raw)
In-Reply-To: <CA+xn8YCJ39D5uC-MdbqYy6FfgNtTZw5Kg2q8esXYqkvysiqL9g@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2188 bytes --]
Here's attempt #2.
-Steve
On Thu, Sep 17, 2015 at 12:44 PM, Steve Sprang <steve.sprang@gmail.com> wrote:
> On Thu, Sep 17, 2015 at 11:18 AM, Mathieu Lirzin <mthl@openmailbox.org> wrote:
>> Steve Sprang <steve.sprang@gmail.com> writes:
>>
>>> From 11440f61cebd49d31d165f6433ec6e4b1afe728f Mon Sep 17 00:00:00 2001
>>> From: Steve Sprang <scs@stevesprang.com>
>>> Date: Thu, 17 Sep 2015 04:22:01 -0700
>>> Subject: [PATCH] download: Fix some minor progress-logging regressions.
>>>
>>> * guix/build/download.scm
>>> (string-pad-middle): Allow resulting padded string to overflow.
>>> (store-url-abbreviation): Remove unnecessary procedure.
>>> (progress-proc): Default abbreviation should be basename.
>>
>> What about something like "(progress-proc): Use BASENAME as default for
>> parameter 'abbreviation'." ?
>
> Ok, sounds good.
>
>>> (url-fetch): Insert some newlines for readability.
>>
>> I would prefer "Display" instead of "Insert" to not confuse with
>> insertion of newlines in the source code (even if such change won't be
>> notified in the commit log) :)
>
> Ok.
>
>>> ---
>>> guix/build/download.scm | 20 +++++++++-----------
>>> 1 file changed, 9 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/guix/build/download.scm b/guix/build/download.scm
>>> index 9b72e8f..e6de4d2 100644
>>> --- a/guix/build/download.scm
>>> +++ b/guix/build/download.scm
>>> @@ -100,15 +100,13 @@ width of the bar is BAR-WIDTH."
>>>
>>> (define (string-pad-middle left right len)
>>> "Combine LEFT and RIGHT with enough padding in the middle so that the
>>> -resulting string has length at least LEN. This right justifies RIGHT."
>> ^^^
>> Maybe it's only because of my poor english but I don't understand the
>> meaning of the last sentence.
>
> Yes, the phrasing is awkward. Basically the effect is that the RIGHT
> string will be flush with the right edge when the string is LEN width
> (i.e. it is right-justified as in typography). I will try to phrase it
> better.
>
> I have to head out for the moment, but I will send a tweaked patch later today.
>
> -Steve
>
>> Otherwise this looks good to me.
>>
>> --
>> Mathieu Lirzin
[-- Attachment #2: progress-fix-2.patch --]
[-- Type: text/x-patch, Size: 2764 bytes --]
From 65fe041c63a58ae5c72ef33f05349ca04afa470b Mon Sep 17 00:00:00 2001
From: Steve Sprang <scs@stevesprang.com>
Date: Thu, 17 Sep 2015 04:22:01 -0700
Subject: [PATCH] download: Fix some minor progress-logging regressions.
* guix/build/download.scm
(string-pad-middle): Allow resulting padded string to overflow.
(store-url-abbreviation): Remove unnecessary procedure.
(progress-proc): Use BASENAME as default for parameter 'abbreviation'.
(url-fetch): Display extra newlines for readability.
---
guix/build/download.scm | 21 ++++++++++-----------
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/guix/build/download.scm b/guix/build/download.scm
index d362fc1..be4a581 100644
--- a/guix/build/download.scm
+++ b/guix/build/download.scm
@@ -101,15 +101,14 @@ width of the bar is BAR-WIDTH."
(define (string-pad-middle left right len)
"Combine LEFT and RIGHT with enough padding in the middle so that the
-resulting string has length at least LEN. This right justifies RIGHT."
- (string-append left
- (string-pad right (max 0 (- len (string-length left))))))
-
-(define (store-url-abbreviation url)
- "Return a friendlier version of URL for display."
- (let ((store-path (string-append (%store-directory) "/" (basename url))))
- ;; Take advantage of the implementation for store paths.
- (store-path-abbreviation store-path)))
+resulting string has length at least LEN (it may overflow). If the string
+does not overflow, the last char in RIGHT will be flush with the LEN
+column."
+ (let* ((total-used (+ (string-length left)
+ (string-length right)))
+ (num-spaces (max 1 (- len total-used)))
+ (padding (make-string num-spaces #\space)))
+ (string-append left padding right)))
(define* (store-path-abbreviation store-path #:optional (prefix-length 6))
"Return an abbreviation of STORE-PATH for display, showing PREFIX-LENGTH
@@ -121,7 +120,7 @@ characters of the hash."
(define* (progress-proc file size
#:optional (log-port (current-output-port))
- #:key (abbreviation identity))
+ #:key (abbreviation basename))
"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
`dump-port'. The progress report is written to LOG-PORT, with ABBREVIATION
@@ -519,7 +518,7 @@ on success."
(_ (list (string->uri url))))))
(define (fetch uri file)
- (format #t "starting download of `~a' from `~a'...~%"
+ (format #t "~%Starting download of ~a~%From ~a...~%"
file (uri->string uri))
(case (uri-scheme uri)
((http https)
--
2.5.0
next prev parent reply other threads:[~2015-09-17 20:52 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-09-17 11:24 [PATCH] download: Fix some minor progress-logging regressions Steve Sprang
2015-09-17 18:18 ` Mathieu Lirzin
2015-09-17 19:44 ` Steve Sprang
2015-09-17 20:52 ` Steve Sprang [this message]
2015-09-17 21:44 ` Mathieu Lirzin
2015-09-24 0:48 ` Steve Sprang
2015-09-24 7:42 ` Ludovic Courtès
2015-09-24 2:05 ` Mark H Weaver
2015-09-17 21:21 ` Mathieu Lirzin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CA+xn8YC788ttPLy-Fi4K20PnVwBr0Lg7GPSE+cjTAYqo5puoZA@mail.gmail.com \
--to=steve.sprang@gmail.com \
--cc=guix-devel@gnu.org \
--cc=mthl@openmailbox.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.