Sending to bug-wget.

On Thu, May 1, 2014 at 4:24 PM, Darshit Shah <[email protected]> wrote:
> On Thu, May 1, 2014 at 3:58 PM, Gisle Vanem <[email protected]> wrote:
>> You wrote:
>>
>>> I honestly haven't looked at the Windows specific part of the codebase
>>> at all, so I don't know how mswindows.c works. I'll however give it a
>>> look when I can.
>>
>>
>> Hi again.
>>
>> I built Wget today from savannah with the updated progress patches.
>> But this new '--show-progress' (or something else?) is a bit broken here on
>> MingW. The 1st character on line >1 are missing. E.g:
>>
>> wget.exe  -q --show-progress -rApng ftp://ftp.watt-32.net/watt-doc/
>>
>> ftp.watt-32.net/watt-doc/.listing     [   <=>                   ] 56,013
>> 116KB/s   in 0.5s
>> tp.watt-32.net/watt-doc/doxygen.p 100%[========================>] 1,576
>> --.-K/s   in 0.003s
>> tp.watt-32.net/watt-doc/ftv2blank 100%[========================>] 174
>> --.-K/s   in 0s
>> tp.watt-32.net/watt-doc/ftv2doc.p 100%[========================>] 255
>> --.-K/s   in 0s
>> tp.watt-32.net/watt-doc/ftv2folde 100%[========================>] 259
>> --.-K/s   in 0s
>> tp.watt-32.net/watt-doc/ftv2folde 100%[========================>] 261
>> --.-K/s   in 0s
>>
>> ---------
> Oh I want to say, "It's not a bug, it's a feature."
> The files you're trying to download have names longer than the maximum
> allowed. So, wget cycles through the characters like a ticker. But
> since your files are small, they download before wget can actually
> tick.
>
> Although I do think the ticker isn't being reset after each file, and
> hence you see the first character missing. Maybe it should be reset.
> I'll look into it.
>
> Also, I don't like how the output is often staggered in this use case.
> I'd like to create more fixed bounds for each column. I'll try working
> on that when I can. Suggestions/patches are more than welcome.
>>
>> The progress is even more broken with 'dot's:
>>  wget.exe  -q --show-progress --progress=dot -rApng
>> ftp://ftp.watt-32.net/watt-doc/
>>
>>     0K .......... .......... .......... .......... ..........  105K
>>    50K ....                                                   2.90M=0.5s
>>
>>
>>     0K .                                                     100%
>> 878K=0.002s
>>
>>
>>     0K                                                       100% 2.51M=0s
>>
>>
>>     0K                                                       100% 3.60M=0s
>>
>>
>>     0K                                                       100% 3.76M=0s
>>
>>
>>     0K                                                       100% 3.89M=0s
>>
>>
>>     0K                                                       100% 3.30M=0s
>>
>> ------------
>>
>> (3 empty lines for each file).
>>
> I'm not sure. I didn't touch the implementation of the dot progress
> bar. When I tried it with a large file, it seemed fine. Maybe I should
> have run my tests with a bunch of small files too. This, IMHO, is only
> a formatting problem. The file is downloaded much before Wget gets a
> chance to print all the dots.
>
> The excessive blank lines are my fault. I cleared those from the bar
> progress, but forgot to do the same from the dots style too.
> Those lines are printed to format the original verbose output, but
> instead only the blank lines get printed in the quiet version. I'll
> fix that.
>
> I can confirm that this indeed is the case because I ran wget compiled
> against v1.15 and compared the output. The individual progress bars
> pretty much line up exactly.
>
>> Besides, the Windows title-bar is missing with '-q --show-progress' now. I
>> don't
>> think that was the idea. The reason is that:
>>  #ifdef WINDOWS
>>          ws_changetitle (hurl);
>>  #endif
>>
>> is called only if 'opt.verbose' is true. IMHO the line:
>> char *hurl = url_string (u, URL_AUTH_HIDE_PASSWD);
>>
>> should set a global WINDOWS-only string/buffer just for these cases. Hence
>> no need to call ws_changetitle(hurl). Just let ws_percenttitle() use this
>> global string. How about it?
>>
> I'm not a fan of global strings/buffers and would like to avoid adding
> new ones. However, looking at the code, your approach seems like the
> cleanest. Does anyone else have better ideas??
>
> Also, I'd request you to make the changes and submit a patch for two
> major reasons:
> 1. I'm not very thorough with the Windows-specific code and don't know
> about what calls what from where.
> 2. I don't have a Windows machine on hand for testing and I'm afraid
> of breaking things without.
>
> To add to it all, I'm keeping a little too busy these days with my
> academic work too.

Please do check the attached patch. It should fix the erroneous empty
lines you reported.

-- 
Thanking You,
Darshit Shah
From e54afa54011e5e528e9660ebdc11a0f53643eae7 Mon Sep 17 00:00:00 2001
From: Darshit Shah <[email protected]>
Date: Thu, 1 May 2014 16:30:10 +0200
Subject: [PATCH] Fix extra newlines when not in verbose mode

---
 src/ChangeLog  | 5 +++++
 src/progress.c | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/ChangeLog b/src/ChangeLog
index 96723dd..797d8b2 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,8 @@
+2014-05-01  Darshit Shah  <[email protected]> (tiny change)
+
+	* progress.c (dot_finish): Do not print extra newlines when not in verbose
+	mode. (Purely aesthetic change)
+
 2014-04-19  Darshit Shah  <[email protected]>
 
 	* log.h (log_options): Add new logging options, LOG_PROGRESS. All progress
diff --git a/src/progress.c b/src/progress.c
index b1d5095..35c4460 100644
--- a/src/progress.c
+++ b/src/progress.c
@@ -407,7 +407,7 @@ dot_finish (void *progress, double dltime)
     }
 
   print_row_stats (dp, dltime, true);
-  logputs (LOG_PROGRESS, "\n\n");
+  logputs (LOG_VERBOSE, "\n\n");
   log_set_flush (false);
 
   xfree (dp);
-- 
1.9.2

Reply via email to