all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* [PATCH] download: Fix some minor progress-logging regressions.
@ 2015-09-17 11:24 Steve Sprang
  2015-09-17 18:18 ` Mathieu Lirzin
  0 siblings, 1 reply; 9+ messages in thread
From: Steve Sprang @ 2015-09-17 11:24 UTC (permalink / raw)
  To: guix-devel


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

Just some small tweaks to fix up logging.

-Steve

[-- Attachment #1.2: Type: text/html, Size: 92 bytes --]

[-- Attachment #2: progress-fix.patch --]
[-- Type: text/x-patch, Size: 2683 bytes --]

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.
  (url-fetch): Insert some newlines for readability.
---
 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."
-  (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).  This right
+justifies RIGHT."
+  (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
@@ -120,7 +118,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
@@ -518,7 +516,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


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

* Re: [PATCH] download: Fix some minor progress-logging regressions.
  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
  0 siblings, 1 reply; 9+ messages in thread
From: Mathieu Lirzin @ 2015-09-17 18:18 UTC (permalink / raw)
  To: Steve Sprang; +Cc: guix-devel

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'." ?

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

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

Otherwise this looks good to me.

--
Mathieu Lirzin

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

* Re: [PATCH] download: Fix some minor progress-logging regressions.
  2015-09-17 18:18 ` Mathieu Lirzin
@ 2015-09-17 19:44   ` Steve Sprang
  2015-09-17 20:52     ` Steve Sprang
  2015-09-17 21:21     ` Mathieu Lirzin
  0 siblings, 2 replies; 9+ messages in thread
From: Steve Sprang @ 2015-09-17 19:44 UTC (permalink / raw)
  To: Mathieu Lirzin; +Cc: guix-devel

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

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

* Re: [PATCH] download: Fix some minor progress-logging regressions.
  2015-09-17 19:44   ` Steve Sprang
@ 2015-09-17 20:52     ` Steve Sprang
  2015-09-17 21:44       ` Mathieu Lirzin
  2015-09-24  2:05       ` Mark H Weaver
  2015-09-17 21:21     ` Mathieu Lirzin
  1 sibling, 2 replies; 9+ messages in thread
From: Steve Sprang @ 2015-09-17 20:52 UTC (permalink / raw)
  To: Mathieu Lirzin; +Cc: guix-devel

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


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

* Re: [PATCH] download: Fix some minor progress-logging regressions.
  2015-09-17 19:44   ` Steve Sprang
  2015-09-17 20:52     ` Steve Sprang
@ 2015-09-17 21:21     ` Mathieu Lirzin
  1 sibling, 0 replies; 9+ messages in thread
From: Mathieu Lirzin @ 2015-09-17 21:21 UTC (permalink / raw)
  To: Steve Sprang; +Cc: guix-devel

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

> On Thu, Sep 17, 2015 at 11:18 AM, Mathieu Lirzin <mthl@openmailbox.org> wrote:
>> Steve Sprang <steve.sprang@gmail.com> writes:
>>>  (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.

Ahh! This makes sense finally.  Thanks for the explanation!

--
Mathieu Lirzin

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

* Re: [PATCH] download: Fix some minor progress-logging regressions.
  2015-09-17 20:52     ` Steve Sprang
@ 2015-09-17 21:44       ` Mathieu Lirzin
  2015-09-24  0:48         ` Steve Sprang
  2015-09-24  2:05       ` Mark H Weaver
  1 sibling, 1 reply; 9+ messages in thread
From: Mathieu Lirzin @ 2015-09-17 21:44 UTC (permalink / raw)
  To: Steve Sprang; +Cc: guix-devel

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

> Here's attempt #2.
[...]
> @@ -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)))

The phrasing is fine for me.  Thanks again.

--
Mathieu Lirzin

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

* Re: [PATCH] download: Fix some minor progress-logging regressions.
  2015-09-17 21:44       ` Mathieu Lirzin
@ 2015-09-24  0:48         ` Steve Sprang
  2015-09-24  7:42           ` Ludovic Courtès
  0 siblings, 1 reply; 9+ messages in thread
From: Steve Sprang @ 2015-09-24  0:48 UTC (permalink / raw)
  To: Mathieu Lirzin; +Cc: guix-devel

This patch is still pending. I don't want it to fall through the cracks. :-)

Thanks!
-Steve

On Thu, Sep 17, 2015 at 2:44 PM, Mathieu Lirzin <mthl@openmailbox.org> wrote:
> Steve Sprang <steve.sprang@gmail.com> writes:
>
>> Here's attempt #2.
> [...]
>> @@ -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)))
>
> The phrasing is fine for me.  Thanks again.
>
> --
> Mathieu Lirzin

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

* Re: [PATCH] download: Fix some minor progress-logging regressions.
  2015-09-17 20:52     ` Steve Sprang
  2015-09-17 21:44       ` Mathieu Lirzin
@ 2015-09-24  2:05       ` Mark H Weaver
  1 sibling, 0 replies; 9+ messages in thread
From: Mark H Weaver @ 2015-09-24  2:05 UTC (permalink / raw)
  To: Steve Sprang; +Cc: guix-devel

Steve Sprang <steve.sprang@gmail.com> writes:
> Here's attempt #2.

[...]

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

Pushed, thanks!

     Mark

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

* Re: [PATCH] download: Fix some minor progress-logging regressions.
  2015-09-24  0:48         ` Steve Sprang
@ 2015-09-24  7:42           ` Ludovic Courtès
  0 siblings, 0 replies; 9+ messages in thread
From: Ludovic Courtès @ 2015-09-24  7:42 UTC (permalink / raw)
  To: Steve Sprang; +Cc: guix-devel

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

> This patch is still pending. I don't want it to fall through the cracks. :-)

Thanks for pinging, it had somehow disappeared from my radar.

Ludo’.

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

end of thread, other threads:[~2015-09-24  7:42 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
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

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.