Re: [evolution-patches] IDN patch for evolution head

[Not Zed <notzed ximian com>]

On Sun, 2004-01-04 at 19:57 -0500, Jeffrey Stedfast wrote:
> On Wed, 2003-12-31 at 15:16, Suresh Chandrasekharan wrote:
> > Pl. use this patch.evolution-1.5, this one contains updated configure.in
> > changes. All the others remain same.
> > 
> > Regards,
> > Suresh
> > 
> > 2003/12/28 (日) 20:38 に Suresh Chandrasekharan さんは書きました:
> > > Hi All,
> > >     Attached are the patches for idnkit library, evolution-1.5, have
> > > incorporated rodo's feedback and have made the changes.
> > > 
> > > Pl. check whether these are OK.
> > > 
> > > Thanks,
> > > Suresh
>
> please post patches with the disposition set to inline so we can reply
> and add inline comments
>
> @@ -344,6 +346,7 @@
>  {
>         const char *method = NULL;
>         char *url;
> +       char *t = NULL;
>         
>
> why 't'? why not 'host'? host makes a MUCH better variable name.
>
>
> +#include "config.h"
>  #include <netdb.h>
>
>
> that should be:
>
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
>  #include <netdb.h>
>
> CamelUrlScanner *
>  camel_url_scanner_new (void)
> @@ -209,10 +213,8 @@
>                                 inptr++;
>                         else
>                                 break;
> -                       
>                         while (inptr < inend && is_domain (*inptr))
>                                 inptr++;
> -                       
>                         if (inptr < inend && *inptr == '.' && is_domain
> (inptr[1]))
>                                 inptr++;
>
> don't just randomly remove blank lines.
>
> +#ifdef ENABLE_IDN
> +static gboolean
> +is_idn_domain ( const char **inptr, const char *inend) {
>
> make this into:
>
> static gboolean
> is_idn_domain (const char **inptr, const char *inend)
> {
>
> function open braces start on their own line.
>
> -                       
> +
>
> don't go changing the spacing here. leave it filled with tabs.
>
> for camel-url, utf8_host should ALWAYS be set, even if IDN isn't being
> used. this makes it easier to use and removes the need for all your
> DISP_HOST() macros from all over the place. Actually, better idea.
> url->host should be the utf8_host and there should be a idn_host member
> that has the encoded host name so that no code outside of camel-url
> needs to be changed.
Why do you need the idn_host parameter anyway?  Isn't it only used when formatting/encoding the url?

> resend when you get that far I guess.

Also, camel-url should really be standalone, and not rely on e-uri.