Re: [Bug-wget] [ PATCH ] LIST changes (ver. 3)

[Giuseppe Scrivano]

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