[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r15343 - in trunk/subversion: libsvn_subr

From: Peter N. Lundblad <peter_at_famlundblad.se>
Date: 2005-07-17 23:40:54 CEST

On Fri, 15 Jul 2005 kou@tigris.org wrote:

> Extract NLS initialization code from svn_cmdline_init.
>
> * subversion/libsvn_subr/nls.c
> (svn_nls_environment_init): Initialize NLS environment.
> (svn_nls_init): Initialize NLS environment and set up
> text domain.
>
These are new functions. YOu should say so in the log.

> Added: trunk/subversion/include/svn_nls.h
> Url: http://svn.collab.net/viewcvs/svn/trunk/subversion/include/svn_nls.h?rev=15343
> ==============================================================================
> --- (empty file)
> +++ trunk/subversion/include/svn_nls.h Fri Jul 15 00:06:55 2005
> @@ -0,0 +1,57 @@
> +/**
> + * @copyright
> + * ====================================================================
> + * Copyright (c) 2000-2005 CollabNet. All rights reserved.
> + *
> + * This software is licensed as described in the file COPYING, which
> + * you should have received as part of this distribution. The terms
> + * are also available at http://subversion.tigris.org/license-1.html.
> + * If newer versions of this license are posted there, you may use a
> + * newer version instead, at your option.
> + *
> + * This software consists of voluntary contributions made by many
> + * individuals. For exact contribution history, see the revision
> + * history and logs, available at http://subversion.tigris.org/.
> + * ====================================================================
> + * @endcopyright
> + *
> + * @file svn_nls.h
> + * @brief Support functions for NLS programs
> + */
> +
> +
> +
> +#ifndef SVN_NLS_H
> +#define SVN_NLS_H
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif /* __cplusplus */
> +
> +/** Set up the NLS environment.
> + * Return the error @c APR_EINVAL or APR_INCOMPLETE if an

                                        ^ Missing @c.

> + * error occurs.
> + *
> + * @note This function is for bindings. You should usually
> + * use @c svn_cmdline_init() instead of calling this

If I understand corrrectly those who has delved into the doxygen docs, you
shouldn't need @c here, since the parens will automatically create a link.

> + * function directly. This function should be called
> + * after initializing APR.

Missing @since New in 1.3.

> + */
> +svn_error_t *svn_nls_environment_init (void);
> +
> +/** Set up the NLS which includes NLS environment set up.

This docstring doesn't make the difference between this and the above
clear to me.

> + * Return the error @c APR_EINVAL or APR_INCOMPLETE if an

Missing @c (as above).

> + * error occurs.
> + *
> + * @note This function is for bindings. You should usually
> + * use @c svn_cmdline_init() instead of calling this

Same about @c here too.

> + * function directly. This function should be called
> + * after initializing APR.

Missing @since.

> + */
> +svn_error_t *svn_nls_init (void);
> +

OK. So why do we need both these functions? Becuase the last one also
calls textdomain, which sets the default text domain to subversion. Does
this one-liner really deserve a public API? AFAICT we're relying on the
default text domain in two places in cmdline/cmdline (and I didn't find
any place in the other cmdline programs we have). Can we change those two
places from
gettext(...)
to
dgettext(PACKGE_NAME, ...)
we can stop using plain gettext and get rid of that textdomain() call.

> +#ifdef __cplusplus
> +}
> +#endif /* __cplusplus */
> +
> +#endif /* SVN_NLS_H */
>
> Modified: trunk/subversion/libsvn_subr/cmdline.c
> Url: http://svn.collab.net/viewcvs/svn/trunk/subversion/libsvn_subr/cmdline.c?rev=15343&p1=trunk/subversion/libsvn_subr/cmdline.c&p2=trunk/subversion/libsvn_subr/cmdline.c&r1=15342&r2=15343
> ==============================================================================
> --- trunk/subversion/libsvn_subr/cmdline.c (original)
> +++ trunk/subversion/libsvn_subr/cmdline.c Fri Jul 15 00:06:55 2005
> @@ -161,69 +162,19 @@
> up by APR at exit time. */
> pool = svn_pool_create (NULL);
> svn_utf_initialize (pool);
> -
> -#ifdef ENABLE_NLS
> -#ifdef WIN32
> +
> {
> - WCHAR ucs2_path[MAX_PATH];
> - char* utf8_path;
> - const char* internal_path;
> - apr_pool_t* pool;
> - apr_status_t apr_err;
> - apr_size_t inwords, outbytes, outlength;
> -
> - apr_pool_create (&pool, 0);
> - /* get exe name - our locale info will be in '../share/locale' */
> - inwords = GetModuleFileNameW (0, ucs2_path,
> - sizeof (ucs2_path) / sizeof(ucs2_path[0]));
> - if (! inwords)
> - {
> - /* We must be on a Win9x machine, so attempt to get an ANSI path,
> - and convert it to Unicode. */
> - CHAR ansi_path[MAX_PATH];
> -
> - if (! GetModuleFileNameA (0, ansi_path, sizeof (ansi_path)))
> - goto utf8_error;
> -
> - inwords =
> - MultiByteToWideChar (CP_ACP, 0, ansi_path, -1, ucs2_path,
> - sizeof (ucs2_path) / sizeof (ucs2_path[0]));
> - if (! inwords)
> - goto utf8_error;
> - }
> -
> - outbytes = outlength = 3 * (inwords + 1);
> - utf8_path = apr_palloc (pool, outlength);
> - apr_err = apr_conv_ucs2_to_utf8 (ucs2_path, &inwords,
> - utf8_path, &outbytes);
> - if (!apr_err && (inwords > 0 || outbytes == 0))
> - apr_err = APR_INCOMPLETE;
> - if (apr_err)
> + svn_error_t *error = svn_nls_init();

We use err instead of error if not everywhere, at least very often.

> + if (error)
> {
> - utf8_error:
> if (error_stream)
> - fprintf (error_stream, "Can't convert module path to UTF-8");
> + fprintf(error_stream, "%s", error->message);

Space before paren.

> +

error->message is not guaranteed to be non-NULL.

> + svn_error_clear(error);

Space before paren.

> return EXIT_FAILURE;
> }
> -
> - utf8_path[outlength - outbytes] = '\0';
> - internal_path = svn_path_internal_style (utf8_path, pool);
> - /* get base path name */
> - internal_path = svn_path_dirname (internal_path, pool);
> - internal_path = svn_path_join (internal_path, SVN_LOCALE_RELATIVE_PATH,
> - pool);
> - bindtextdomain (PACKAGE_NAME, internal_path);
> - apr_pool_destroy (pool);
> }
> -#else
> - bindtextdomain(PACKAGE_NAME, SVN_LOCALE_DIR);
> -#ifdef HAVE_BIND_TEXTDOMAIN_CODESET
> - bind_textdomain_codeset(PACKAGE_NAME, "UTF-8");
> -#endif
> -#endif
> - textdomain(PACKAGE_NAME);
> -#endif
> -
> +
> return EXIT_SUCCESS;
> }
>
>
> Added: trunk/subversion/libsvn_subr/nls.c
> Url: http://svn.collab.net/viewcvs/svn/trunk/subversion/libsvn_subr/nls.c?rev=15343
> ==============================================================================
> --- (empty file)
> +++ trunk/subversion/libsvn_subr/nls.c Fri Jul 15 00:06:55 2005
> @@ -0,0 +1,129 @@
Regards,
//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sun Jul 17 23:41:37 2005

This is an archived mail posted to the Subversion Dev mailing list.