I just went through the patch your submitted. I've attached a copy of the
patch file as generated through git format-patch for anyone who may want to
try it out.
There's a few comments I have. Please find them inline.
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);
> - }
>
> -----------
>
> Would it not be a better idea to wrap this segment completely into a
if(opt.show_progress || opt.verbose) block?
That way, the hurl is generated only if it is required.
What I suggest is something along these lines:
diff --git a/src/ftp.c b/src/ftp.c
index c78256f..9315c8f 100644
--- a/src/ftp.c
+++ b/src/ftp.c
@@ -1594,7 +1594,6 @@ ftp_loop_internal (struct url *u, struct fileinfo *f,
ccon *con, char **local_fi
/* THE loop. */
do
{
- char *hurl;
/* Increment the pass counter. */
++count;
@@ -1655,7 +1654,9 @@ ftp_loop_internal (struct url *u, struct fileinfo *f,
ccon *con, char **local_fi
/* Get the current time string. */
tms = datetime_str (time (NULL));
- hurl = url_string (u, URL_AUTH_HIDE_PASSWD);
+ if (opt.show_progress || opt.verbose)
+ {
+ char *hurl = url_string (u, URL_AUTH_HIDE_PASSWD);
/* Print fetch message, if opt.verbose. */
if (opt.verbose)
@@ -1669,10 +1670,11 @@ ftp_loop_internal (struct url *u, struct fileinfo
*f, ccon *con, char **local_fi
}
#ifdef WINDOWS
- if (!opt.quiet || opt.show_progress)
+ if (opt.show_progress)
ws_changetitle (hurl);
#endif
xfree (hurl);
+ }
/* Send getftp the proper length, if fileinfo was provided. */
if (f && f->type != FT_SYMLINK)
There's a few salient points in this I'd like to point out.
As of now, opt.verbose unconditionally imples that opt.show_progress is
set. This is because --no-show-progress does not currently work. However,
we still have the (opt.show_progress || opt.verbose) since, in the future
we may have --no-show-progress working and then the logic here should not
break.
But encapsulating this into a single block, we create and destroy the hurl
only of required.
I've also changed the conditional in the #ifdef WINDOWS section.
According to your original patch, the console title is updated when the
user invokes wget only with --no-verbose too. This is contrary to what I'd
expect since the progress bar isn't displayed in that use case. If I
understand correctly, you only want to update the console title when the
progress bar is being displayed, we only need check opt.show_progress for
that. Or did I miss something?
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:
>
> 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 c38b1a2650ebf4b0b0c7b3aefe6982d52cef3507 Mon Sep 17 00:00:00 2001
From: Darshit Shah <[email protected]>
Date: Sat, 3 May 2014 08:48:00 +0200
Subject: [PATCH] Display progress information in Windows console title ba
---
src/ChangeLog | 9 +++++++++
src/ftp.c | 15 +++++++++++----
src/http.c | 12 ++++++++----
src/retr.c | 2 +-
4 files changed, 29 insertions(+), 9 deletions(-)
diff --git a/src/ChangeLog b/src/ChangeLog
index 92942eb..412f366 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,12 @@
+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.
+
2014-05-01 Darshit Shah <[email protected]> (tiny change)
* progress.c (dot_finish): Do not print extra newlines when not in verbose
diff --git a/src/ftp.c b/src/ftp.c
index 2db98bb..c78256f 100644
--- a/src/ftp.c
+++ b/src/ftp.c
@@ -1594,6 +1594,8 @@ ftp_loop_internal (struct url *u, struct fileinfo *f, ccon *con, char **local_fi
/* THE loop. */
do
{
+ char *hurl;
+
/* Increment the pass counter. */
++count;
sleep_between_retrievals (count);
@@ -1652,21 +1654,26 @@ ftp_loop_internal (struct url *u, struct fileinfo *f, ccon *con, char **local_fi
/* 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
- ws_changetitle (hurl);
+ if (!opt.quiet || opt.show_progress)
+ ws_changetitle (hurl);
#endif
- xfree (hurl);
- }
+ xfree (hurl);
+
/* Send getftp the proper length, if fileinfo was provided. */
if (f && f->type != FT_SYMLINK)
len = f->size;
diff --git a/src/http.c b/src/http.c
index 9551fb1..0d5217d 100644
--- a/src/http.c
+++ b/src/http.c
@@ -3088,6 +3088,8 @@ http_loop (struct url *u, struct url *original_url, char **newloc,
/* THE loop */
do
{
+ char *hurl;
+
/* Increment the pass counter. */
++count;
sleep_between_retrievals (count);
@@ -3099,10 +3101,11 @@ http_loop (struct url *u, struct url *original_url, char **newloc,
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)
{
@@ -3116,12 +3119,13 @@ Spider mode enabled. Check if remote file exists.\n"));
logprintf (LOG_NOTQUIET, "--%s-- %s\n",
tms, hurl);
}
+ }
#ifdef WINDOWS
- ws_changetitle (hurl);
+ if (!opt.quiet || opt.show_progress)
+ ws_changetitle (hurl);
#endif
- xfree (hurl);
- }
+ xfree (hurl);
/* Default document type is empty. However, if spider mode is
on or time-stamping is employed, HEAD_ONLY commands is
diff --git a/src/retr.c b/src/retr.c
index 135f599..8fe12ad 100644
--- a/src/retr.c
+++ b/src/retr.c
@@ -412,7 +412,7 @@ fd_read_body (const char *downloaded_filename, int fd, FILE *out, wgint toread,
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
--
1.9.2
