Thanks for your comments. I suspect that I accidentally deleted the lines when I merged in the changes from the mainline into my branch - it's my first time using bzr.
I will correct the mistakes and resubmit the patch for reconsideration. Lenni On Tue, Jan 25, 2011 at 5:10 PM, Giuseppe Scrivano <[email protected]>wrote: > Hi Leonard, > > Leonard Ehrenfried <[email protected]> writes: > > > I'd be greatful for a ruthless code review - it's the only way to learn! > > I have some comments about your patch: > > > > === modified file 'src/main.c' > > --- src/main.c 2011-01-01 12:19:37 +0000 > > +++ src/main.c 2011-01-25 14:35:05 +0000 > > @@ -54,7 +54,6 @@ > > #include "convert.h" > > #include "spider.h" > > #include "http.h" /* for save_cookies */ > > -#include "ptimer.h" > > Why are you removing this line? > > > > > #include <getopt.h> > > #include <getpass.h> > > @@ -164,7 +163,6 @@ > > { IF_SSL ("certificate-type"), 0, OPT_VALUE, "certificatetype", -1 > }, > > { IF_SSL ("check-certificate"), 0, OPT_BOOLEAN, "checkcertificate", > -1 }, > > { "clobber", 0, OPT__CLOBBER, NULL, optional_argument }, > > - { "config", 0, OPT_VALUE, "chooseconfig", -1 }, > > Same here. > > > > > - N_("\ > > - --config=FILE Specify config file to use.\n"), > > And here. > > > > > - struct ptimer *timer = ptimer_new (); > > [....] > > - xfree (wall_time); > > - xfree (download_time); > > > And this whole block? > > Please fix your patch to don't modify code which is not part of your patch. > > > > > === modified file 'src/options.h' > > --- src/options.h 2011-01-01 12:19:37 +0000 > > +++ src/options.h 2011-01-25 14:35:05 +0000 > > @@ -59,6 +59,7 @@ > > char *dir_prefix; /* The top of directory tree */ > > char *lfilename; /* Log filename */ > > char *input_filename; /* Input filename */ > > + bool purge_input_file; /* Remove donwloaded URLs from the input > file */ > > char *choose_config; /* Specified config file */ > > bool force_html; /* Is the input file an HTML file? */ > > > > > > === modified file 'src/retr.c' > > --- src/retr.c 2011-01-01 12:19:37 +0000 > > +++ src/retr.c 2011-01-25 14:35:05 +0000 > > @@ -922,7 +922,7 @@ > > If opt.recursive is set, call retrieve_tree() for each file. */ > > > > uerr_t > > -retrieve_from_file (const char *file, bool html, int *count) > > +retrieve_from_file (const char *file, bool html, bool purge, int *count) > > { > > uerr_t status; > > struct urlpos *url_list, *cur_url; > > @@ -943,6 +943,7 @@ > > int dt,url_err; > > uerr_t status; > > struct url * url_parsed = url_parse(url, &url_err, iri, true); > > + > > > > if (!url_parsed) > > { > > @@ -987,6 +988,7 @@ > > > > for (cur_url = url_list; cur_url; cur_url = cur_url->next, ++*count) > > { > > + > > char *filename = NULL, *new_file = NULL; > > int dt; > > struct iri *tmpiri = iri_dup (iri); > > @@ -1024,6 +1026,62 @@ > > cur_url->url->url, &filename, > > &new_file, NULL, &dt, opt.recursive, > tmpiri, > > true); > > + /* > > + * --purge-input-file handling > > + */ > > + if(RETROK == status && purge) > > + { > > + FILE *infile; > > + infile = fopen( input_file, "r"); > > + char line[2000]; > > Please remove this magic number and use instead the readline module > offered by gnulib. > > Please format all your code following the GNU coding standards: > > http://www.gnu.org/prep/standards/standards.html#Formatting > > > I would like to release wget as soon as possible, I don't think we will > be able to include your patch in the next wget release; the good thing > is that you'll have more time to work on it ;-) > > I'll send you in another e-mail details to start the copyright > assignment process to the FSF (the main reason why I think we will not > be able to include your patch in the next release...). > > Cheers, > Giuseppe >
