Kouhei Sutou wrote:
> No, there are any reason. I just extracted.
>
> How about the attached patch?
Looks pretty good. I've included a few comments inline.
> In the patch, svn_nls_environment_init() and svn_nls_init()
> are return svn_error_t * and receive no
> arguments. svn_cmdline_init() displays error messages
> generated by them to error_stream if error_stream is not
> NULL.
>
>
> Thanks,
> --
> kou
>
>
> ------------------------------------------------------------------------
>
> Index: subversion/include/svn_nls.h
> ===================================================================
> --- subversion/include/svn_nls.h (revision 0)
> +++ subversion/include/svn_nls.h (revision 0)
> @@ -0,0 +1,51 @@
> +/**
> + * @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 fail.
Rather than "if fail", you might want to say something like "if an error
occurs", just more correct english, not a big deal.
> + *
> + * @note This function is for bindings. usually use svn_cmdline_init().
Maybe something like: "You should usually use @c svn_cmdline_init()
instead of calling this function directly." Again, this is just a bit
of wordsmithing, nothing wrong with the content of what you said, just
how you said it could use some work.
> + * This function should be called after initializing APR.
> + */
> +svn_error_t *svn_nls_environment_init (void);
> +
> +/** Set up the NLS which includes NLS environment set up.
> + * Return the error @c APR_EINVAL or APR_INCOMPLETE if fail.
Same thing here about "if fail".
> + *
> + * @note This function is for bindings. usually use svn_cmdline_init().
> + * This function should be called after initializing APR.
> + */
> +svn_error_t *svn_nls_init (void);
> +
> +#ifdef __cplusplus
> +}
> +#endif /* __cplusplus */
> +
> +#endif /* SVN_NLS_H */
> Index: subversion/libsvn_subr/cmdline.c
> ===================================================================
> --- subversion/libsvn_subr/cmdline.c (revision 15309)
> +++ subversion/libsvn_subr/cmdline.c (working copy)
> @@ -35,6 +35,7 @@
> #include "svn_path.h"
> #include "svn_pools.h"
> #include "svn_error.h"
> +#include "svn_nls.h"
> #include "utf_impl.h"
>
> #include "svn_private_config.h"
> @@ -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)
> + svn_error_t *error = svn_nls_init();
> + if (error != SVN_NO_ERROR)
No reason to explicitly compare against SVN_NO_ERROR, it's common enough
in the rest of the svn codebase to just do 'if (error)'.
> {
> - /* 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)
> - {
> - utf8_error:
> if (error_stream)
> - fprintf (error_stream, "Can't convert module path to UTF-8");
> + fprintf(error_stream, error->message);
It's generally bad form to pass anything other than a constant string as
the format string argument to fprintf. Consider what would happen if
error->message had %s (or some other format string) in it? Something like
fprintf(error_stream, "%s", error->message);
would be safer.
> +
> + svn_error_clear(error);
> 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;
> }
>
> Index: subversion/libsvn_subr/nls.c
> ===================================================================
> --- subversion/libsvn_subr/nls.c (revision 0)
> +++ subversion/libsvn_subr/nls.c (revision 0)
> @@ -0,0 +1,129 @@
> +/*
> + * nls.c : Helpers for NLS programs.
> + *
> + * ====================================================================
> + * 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/.
> + * ====================================================================
> + */
> +
> +#include <stdlib.h>
> +
> +#ifndef WIN32
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
> +#include <unistd.h>
> +#endif
> +
> +#include <apr_errno.h>
> +
> +#include "svn_error.h"
> +#include "svn_pools.h"
> +
> +#include "svn_private_config.h"
> +
> +
> +svn_error_t *
> +svn_nls_environment_init (void)
> +{
> + svn_error_t *error = SVN_NO_ERROR;
> +
> +#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)))
> + {
> + inwords =
> + MultiByteToWideChar (CP_ACP, 0, ansi_path, -1, ucs2_path,
> + sizeof (ucs2_path) / sizeof (ucs2_path[0]));
> + if (! inwords) {
> + error =
> + svn_error_createf(APR_EINVAL, NULL,
> + _("Can't convert string to UCS-2: '%s'"),
> + ansi_path);
> + }
> + }
> + else
> + {
> + error = svn_error_create(APR_EINVAL, NULL,
> + _("Can't get module file name"));
> + }
> + }
> +
> + if (error == SVN_NO_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)
> + {
> + error = svn_error_create(apr_err, NULL,
> + _("Can't convert module path "
> + "to UTF-8 from UCS-2: '%s'"),
> + ucs2_path);
> + }
> + else
> + {
> + 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
> +#endif
> +
> + return error;
> +}
> +
> +svn_error_t *
> +svn_nls_init (void)
> +{
> +#ifdef ENABLE_NLS
> + SVN_ERR(svn_nls_environment_init());
> + textdomain(PACKAGE_NAME);
> +#endif
> +
> + return SVN_NO_ERROR;
> +}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sun Jul 10 07:32:00 2005