[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Bug-wget] [ PATCH ] LIST changes (ver. 3)
From: |
Giuseppe Scrivano |
Subject: |
Re: [Bug-wget] [ PATCH ] LIST changes (ver. 3) |
Date: |
Fri, 18 Oct 2013 17:41:43 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) |
Tim Ruehsen <address@hidden> writes:
> Hi Andrea,
>
> good work !
>
> I could apply the new patch, the test suite runs smoothly.
> Some recursive tests on random ftp servers not in your list worked well.
>
> There is one little thing:
> ftp.c:1496:21: warning: 'previous_rd_size' may be used uninitialized in this
> function [-Wmaybe-uninitialized]
> else if ( previous_rd_size > rd_size )
>
> Guiseppe might have some (indentation) remarks.
thanks to have done the real review :-) so now the boring part:
> From 46fbb84ed2eb67079151c86b949c7b087d9d58be Mon Sep 17 00:00:00 2001
> From: Andrea Urbani <address@hidden>
> Date: Fri, 18 Oct 2013 09:29:13 +0200
> Subject: [PATCH] Now wget, after the SYST command, looks if it knows that
> system. If yes, wget will force the use of "LIST" or "LIST -a". If no, wget
> will try, only the first time of each session, before the "LIST -a" command
> and after the "LIST". If "LIST -a" works and returns more or equal data of
> the "LIST", "LIST -a" will be the standard list command for all the session.
> If "LIST -a" fails or returns less data than "LIST" (think on the case of an
> existing file called "-a"), "LIST" will be the standard list command for all
> the session.
please split this huge line, the first line should be a summary of the
patch.
> - if (rs == ST_VMS)
> + if ( avoid_list_a )
> + {
> i = countof (list_commands)- 1;
> + DEBUGP (("(skipping \"LIST -a\")"));
> + }
> +
please use two spaces to indent the {} block as, also no empty spaces to
wrap the argument to if. The same remark applies to other cases in the
patch.
if (avoid_list_a)
{
i = countof (list_commands)- 1;
DEBUGP (("(skipping \"LIST -a\")"));
}
> struct fileinfo *ftp_parse_ls (const char *, const enum stype);
> diff --git a/tests/ChangeLog b/tests/ChangeLog
> index fab7460..f152c32 100644
> --- a/tests/ChangeLog
> +++ b/tests/ChangeLog
> @@ -1,3 +1,52 @@
> +2013-10-17 Andrea Urbani <address@hidden>
> + * FTPServer.pm:
> + - package FTPPaths, added sub GetBehavior: It returns the
> + behavior of the given name
> + - new server behaviors:
> + list_dont_clean_path : if defined, the command
> + $path =~ s/^-[a-zA-Z0-9]+\s?//;
> + is not runt and the given path
> + remains the original one
> + list_empty_if_list_a : if defined, "LIST -a" returns an
> + empty content
> + list_fails_if_list_a : if defined, "LIST -a" returns an
> + error
> + list_no_hidden_if_list: if defined, "LIST" doesn't return
> + hidden files.
> + To define an hidden file add
> + attr => "H"
> + to the url files
> + syst_response : if defined, its content is printed
> + out as SYST response
please follow the ChangeLog format here, we don't really require so many
details, just a brief description of what you changed..
+ * FTPServer.pm (GetBehavior): new routine.
this can be enough to replace the entire block.
Please also remove the trailing white spaces introduced by your patch, I
usually run "delete-trailing-whitespaces" in my Emacs buffer to be sure
I didn't leave anything there.
Let me know if you have questions or doubts.
Regards,
Giuseppe