Re: [Bug-wget] --progress should not be overridden by --quiet

[Darshit Shah]

On Thu, May 1, 2014 at 5:44 PM, Gisle Vanem <address@hidden> wrote:
>> 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.
>
>
> Looks like so, yes. With normal sized files, I see the filename scroll
> from right to left. Pretty cool.
>
Attached is a new version of the patch. I have also fixed some
off-by-one errors in the progress bar implementation that had long
gone unnoticed, becuase progress bars were never printed one under
another.

The ugly output you saw earlier with the size of the progress bars
varying and other columns out of sync, should be fixed now.

I really urge people to test this patch and see if you like it. The
patch applies directly on origin/master without the previous patch.

>
>> 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??
>
>
> Here is a patch that doesn't use any global data. My idea is that creating +
> free'ing a 'hurl' is inexpensive in terms of CPU and memory. So it could be
> done irrespective of 'opt.verbose'. Like so:
>
> diff -Hb -u3 ../Git-latest/src/ftp.c ./ftp.c
> --- ../Git-latest/src/ftp.c     2014-05-01 14:54:39 +0000
> +++ ./ftp.c     2014-05-01 16:11:13 +0000
> @@ -1594,6 +1594,8 @@
>   /* THE loop.  */
>   do
>     {
> +      char *hurl;
> +
>       /* Increment the pass counter.  */
>       ++count;
>       sleep_between_retrievals (count);
> @@ -1652,21 +1654,26 @@
>
>       /* Get the current time string.  */
>       tms = datetime_str (time (NULL));
> +
> +      hurl = url_string (u, URL_AUTH_HIDE_PASSWD);
> +
>       /* Print fetch message, if opt.verbose.  */
>       if (opt.verbose)
>         {
> -          char *hurl = url_string (u, URL_AUTH_HIDE_PASSWD);
>           char tmp[256];
>           strcpy (tmp, "        ");
>           if (count > 1)
>             sprintf (tmp, _("(try:%2d)"), count);
>           logprintf (LOG_VERBOSE, "--%s--  %s\n  %s => %s\n",
>                      tms, hurl, tmp, quote (locf));
> +        }
> +
> #ifdef WINDOWS
> +      if (!opt.quiet || opt.show_progress)
>           ws_changetitle (hurl);
> #endif
>           xfree (hurl);
> -        }
> +
>       /* Send getftp the proper length, if fileinfo was provided.  */
>       if (f && f->type != FT_SYMLINK)
>         len = f->size;
>
> diff -Hb -u3 ../Git-latest/src/http.c ./http.c
> --- ../Git-latest/src/http.c    2014-05-01 14:54:39 +0000
> +++ ./http.c    2014-05-01 16:11:09 +0000
> @@ -3088,6 +3088,8 @@
>   /* THE loop */
>   do
>     {
> +      char *hurl;
> +
>       /* Increment the pass counter.  */
>       ++count;
>       sleep_between_retrievals (count);
> @@ -3099,11 +3101,11 @@
>         logprintf (LOG_VERBOSE, _("\
> Spider mode enabled. Check if remote file exists.\n"));
>
> +      hurl = url_string (u, URL_AUTH_HIDE_PASSWD);
> +
>       /* Print fetch message, if opt.verbose.  */
>       if (opt.verbose)
>         {
> -          char *hurl = url_string (u, URL_AUTH_HIDE_PASSWD);
> -
>           if (count > 1)
>             {
>               char tmp[256];
> @@ -3116,12 +3118,13 @@
>               logprintf (LOG_NOTQUIET, "--%s--  %s\n",
>                          tms, hurl);
>             }
> +        }
>
> #ifdef WINDOWS
> +       if (!opt.quiet || opt.show_progress)
>           ws_changetitle (hurl);
> #endif
>           xfree (hurl);
> -        }
>
> -----------
>
> And the most important diff:
>
> diff -Hb -u3 ../Git-latest/src/retr.c ./retr.c
> --- ../Git-latest/src/retr.c    2014-05-01 14:54:39 +0000
> +++ ./retr.c    2014-05-01 15:54:30 +0000
> @@ -412,7 +412,7 @@
>       if (progress)
>         progress_update (progress, ret, ptimer_read (timer));
> #ifdef WINDOWS
> -      if (toread > 0 && opt.show_progress)
> +      if (toread > 0 && (opt.show_progress || !opt.quiet))
>
>         ws_percenttitle (100.0 *
>                          (startpos + sum_read) / (startpos + toread));
> #endif
>
> -----------
>
> As you see, my local files are not under Git-control. So it's hard for me to
> give you any "git diff" patches. And since I suspect src/Changelog is
> created from a mysterious git-command, I don't know how you want a
> change-entry from me. The 'git-format-patch' won't work here. But here is a
> suggestion:
>
Nah. ChangeLog is manually generated just the way you did. :)

I haven't been able to go through your patch yet, or create a unified
patchusing git format-patch. I'll do it in a while. Thanks for
submitting this!

> 2014-05-01  Gisle Vanem  <address@hidden>
>
>  * ftp.c (ftp_loop_internal): Call url_string() to retrieve a 'hurl' incase
> WINDOWS
>    needs to set the console-title.
>  * http.c (http_loop): Call url_string() to retrieve a 'hurl' incase WINDOWS
>     needs to set the console-title.
>  * retr.c (fd_read_body): Update the read-percentage for WINDOWS if
> '--show-progress' or NOT '--quiet' are set.
>
> But hopefully you get the idea.
>
>
>> Please do check the attached patch. It should fix the erroneous empty
>> lines you reported.
>
>
> Looks fine now.
>
> --gv



-- 
Thanking You,
Darshit Shah

0001-Aesthetic-changes-to-progress-bar.patch
Description: Text Data