Hi Michael,

I did some more testing and realized that some padding was being doubly added to the progress bar which caused it to go beyond the width of the screen and hence cause the newline spam you noticed.

Interestingly, the bug has been around for ages and is only now being triggered. Also, interesting to note was that for the Russian locale as mentioned in this report, the character for a space, ' ' was a multi-byte sequence. This multi-byte sequence caused an extra padding in the string which meant the progress bar overflows into the next line.

I've removed that extra padding and now things should work just fine. I tried testing with ru_RU.UTF-8, en_US.UTF-8, de_DE.UTF-8 and nn_NO.UTF-8 and found no problems.

The attached patch also adds an assert statement which should exist in the code for development purposes only. It is better for Wget to generate newline spam and actually download the file instead of crashing and failing.

On 10/28, Darshit Shah wrote:
On 10/28, Darshit Shah wrote:
Hi Michael,

Thanks a lot for your email.

While we try our best to regression test every new feature, some issues
often seep through the cracks.


I'm traveling so testing this is a little hard for me. But I managed to do some preliminary testing and identified that the given issue is being triggered when run in a different locale and the size being displayed in the progress bar is greater than 1M. As soon as the size being displayed there exceeds the 1M mark, the issue gets triggered and I see a new line being created for each refresh of the progress bar.

I'll probably look into this tomorrow, but this information should help save a few minutes for anyone who tries looking into this issue before I can.

On 28-Oct-2014 2:13 pm, "Michael Shigorin" <[email protected]> wrote:

       Hello,
please do pay attention to regression testing for innovative
bells and especially whistles as the "new and improved" progress
bar quickly becomes annoying if the filename is longer than the
part of the line before percent meter: the new code tries to
scroll it (this alone might be irritating to some including
myself) and for some cases fails to do that properly falling into
"spam out a new line every cycle" in xterm with an UTF-8 locale.

Adding the filename was important for some of the other features that we
are currently planning on for future releases of Wget. Some people want the
filename to scroll, others find it irritating. We set scrolling as the
default, but a new option, --no-scroll has also been added for people who
would prefer the progress bar not have a scrolling filename.
You could set it in your wgetrc file to add the option globally.


e.g.,

$ LANG=ru_RU.UTF-8 wget -c
http://fly.osdn.org.ua/~mike/iso/rescue/live-rescue-20141027-x86_64.iso
--2014-10-28 00:28:44--
http://fly.osdn.org.ua/~mike/iso/rescue/live-rescue-20141027-x86_64.iso
Распознаётся fly.osdn.org.ua (fly.osdn.org.ua)... 212.40.45.6
Подключение к fly.osdn.org.ua (fly.osdn.org.ua)|212.40.45.6|:80...
соединение установлено.
HTTP-запрос отправлен. Ожидание ответа... 200 OK
Длина: 620756992 (592M) [application/octet-stream]
Сохранение в: <<live-rescue-20141027-x86_64.iso>>

-rescue-20141027-x8   0%[                      ]   1,25M  1,57MB/s
rescue-20141027-x86   0%[                      ]   1,65M  1,65MB/s
escue-20141027-x86_   0%[                      ]   2,06M  1,71MB/s
scue-20141027-x86_6   0%[                      ]   2,44M  1,74MB/s
cue-20141027-x86_64   0%[                      ]   2,84M  1,78MB/s
ue-20141027-x86_64.   0%[                      ]   3,26M  1,81MB/s
e-20141027-x86_64.i   0%[                      ]   3,68M  1,84MB/s
^C

I'm currently traveling, but will take a look into what causes this issue
as soon as I can. Having a stable release of Wget is our main priority.
while disabling l18n would result in most of the spam missing:

$ LANG=C wget -c
http://fly.osdn.org.ua/~mike/iso/rescue/live-rescue-20141027-x86_64.iso
converted '
http://fly.osdn.org.ua/~mike/iso/rescue/live-rescue-20141027-x86_64.iso'
(ANSI_X3.4-1968) -> '
http://fly.osdn.org.ua/~mike/iso/rescue/live-rescue-20141027-x86_64.iso'
(UTF-8)
--2014-10-28 00:30:09--
http://fly.osdn.org.ua/~mike/iso/rescue/live-rescue-20141027-x86_64.iso
Resolving fly.osdn.org.ua (fly.osdn.org.ua)... 212.40.45.6
Connecting to fly.osdn.org.ua (fly.osdn.org.ua)|212.40.45.6|:80...
connected.
HTTP request sent, awaiting response... 200 OK
Length: 620756992 (592M) [application/octet-stream]
Saving to: 'live-rescue-20141027-x86_64.iso'

live-rescue-2014102   0%[                      ]   4.55M  1.89MB/s
^C

...but adding the "conversion" spam.

This is being caused due to some other idn fixes. I'll see if it can be
prevented.

Guys, please do RCs or whatever to avoid brown paper bag effects
in products that were mature years ago.  I love you and thank for
your work but please please don't feel cool, people depend on wget.

We would generally release a beta version before making a new release.
However, we were forced to make an urgent release this time around due to a
security issue coming up that had to be fixed immediately.

--
---- WBR, Michael Shigorin / http://altlinux.org
 ------ http://opennet.ru / http://anna-news.info

--- end quoted text ---

--
Thanking You,
Darshit Shah


--- end quoted text ---

--
Thanking You,
Darshit Shah
From 1a29f21bb24c531f450dd59446a0c5a0f7e81110 Mon Sep 17 00:00:00 2001
From: Darshit Shah <[email protected]>
Date: Sat, 1 Nov 2014 05:34:04 +0530
Subject: [PATCH] Remove extra padding from the progress bar

---
 src/ChangeLog  | 7 +++++++
 src/progress.c | 7 ++-----
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/src/ChangeLog b/src/ChangeLog
index 58bf905..214441f 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,10 @@
+2014-11-01  Darshit Shah  <[email protected]>
+
+	* progress.c (create_image): Extra padding for size_grouped_diff has already
+	been added. Do not add that again.
+	(create_image): Assert that the progress bar being drawn is lesser than the
+	size of the screen.
+
 2014-10-29  Peter Meiser <[email protected]> (tiny change)
 
 	* openssl.c (ssl_init) [! OPENSSL_NO_SSL3]: Add guard for OpenSSL
diff --git a/src/progress.c b/src/progress.c
index 5ba542d..f211998 100644
--- a/src/progress.c
+++ b/src/progress.c
@@ -907,10 +907,6 @@ create_image (struct bar_progress *bp, double dl_total_time, bool done)
   char *p = bp->buffer;
   wgint size = bp->initial_length + bp->count;
 
-  const char *size_grouped = with_thousand_seps (size);
-  int size_grouped_len = count_cols (size_grouped);
-  /* Difference between num cols and num bytes: */
-  int size_grouped_diff = strlen (size_grouped) - size_grouped_len;
   int size_grouped_pad; /* Used to pad the field width for size_grouped. */
 
   struct bar_progress_hist *hist = &bp->hist;
@@ -1159,9 +1155,10 @@ create_image (struct bar_progress *bp, double dl_total_time, bool done)
       move_to_end (p);
     }
 
-  while (p - bp->buffer - bytes_cols_diff - size_grouped_diff < bp->width)
+  while (p - bp->buffer - bytes_cols_diff < bp->width)
     *p++ = ' ';
   *p = '\0';
+  assert (count_cols(bp->buffer) <= bp->width);
 }
 
 /* Print the contents of the buffer as a one-line ASCII "image" so
-- 
2.1.2

Attachment: pgpC39325CXt8.pgp
Description: PGP signature

Reply via email to