Re: [PATCH] download: Fix some minor progress-logging regressions.

[Steve Sprang]

On Thu, Sep 17, 2015 at 11:18 AM, Mathieu Lirzin <address@hidden> wrote:
> Steve Sprang <address@hidden> writes:
>
>> From 11440f61cebd49d31d165f6433ec6e4b1afe728f Mon Sep 17 00:00:00 2001
>> From: Steve Sprang <address@hidden>
>> 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