On Thu, May 1, 2014 at 5:44 PM, Gisle Vanem <[email protected]> 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 <[email protected]>
>
> * 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
From a8f5e4fe17ebb3310457c07cd50ec90bf9f0d486 Mon Sep 17 00:00:00 2001
From: Darshit Shah <[email protected]>
Date: Thu, 1 May 2014 19:37:39 +0200
Subject: [PATCH] Aesthetic changes to progress bar
---
src/ChangeLog | 9 +++++++++
src/progress.c | 7 +++----
2 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/src/ChangeLog b/src/ChangeLog
index 96723dd..467cc00 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,12 @@
+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)
+ (get_eta): Add extra space when eta is printed.
+ (create_image): Remove erroneous space from being added to progress bar when
+ filename > MAX_FILENAME_LEN
+ (create_image): Remove extra space before printed download speeds
+
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..4530feb 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);
@@ -823,7 +823,7 @@ get_eta (int *bcd)
{
/* TRANSLATORS: "ETA" is English-centric, but this must
be short, ideally 3 chars. Abbreviate if necessary. */
- static const char eta_str[] = N_(" eta %s");
+ static const char eta_str[] = N_(" eta %s");
static const char *eta_trans;
static int bytes_cols_diff;
if (eta_trans == NULL)
@@ -931,7 +931,6 @@ create_image (struct bar_progress *bp, double dl_total_time, bool done)
offset = ((int) bp->tick) % (orig_filename_len - MAX_FILENAME_LEN);
else
offset = 0;
- *p++ = ' ';
memcpy (p, bp->f_download + offset, MAX_FILENAME_LEN);
p += MAX_FILENAME_LEN;
*p++ = ' ';
@@ -1037,7 +1036,7 @@ create_image (struct bar_progress *bp, double dl_total_time, bool done)
wgint dlquant = hist->total_bytes + bp->recent_bytes;
double dltime = hist->total_time + (dl_total_time - bp->recent_start);
double dlspeed = calc_rate (dlquant, dltime, &units);
- sprintf (p, " %4.*f%s", dlspeed >= 99.95 ? 0 : dlspeed >= 9.995 ? 1 : 2,
+ sprintf (p, "%4.*f%s", dlspeed >= 99.95 ? 0 : dlspeed >= 9.995 ? 1 : 2,
dlspeed, !opt.report_bps ? short_units[units] : short_units_bits[units]);
move_to_end (p);
}
--
1.9.2
