(snip)

Branko, I'm so confused from the entire thread and all those points you are
mentioning.

1. I would like to clarify this change. This wasn't made to fix that
problem in the first place. I noticed two unrelated things which were wrong
(in my opinion) in the utf header file/implementation. First - I wanted to
make SVN_APR_LOCALE_CHARSET and  SVN_APR_DEFAULT_CHARSET not just aliases
to APR, but a complete part of the Subversion API. And then I noticed we
can also remove that include (now I understand this was a breaking change
for backward compatibility).

2. Initially, you've vetoed the whole revision (plus follow-ups). This is
fine. I respect vetoes. But your reasoning was that "apr headers may
change". Fine, but I've clarified that it would still work even if APR's
and SVN's constants will be different in the future.

3. It seemed that you agreed with this. However, backward compatibility was
broken by removing apr_xlate.h from svn_utf.h.

4. I thought those were the only mistakes in that commit, affected by the
veto. I reverted this change without changing the remaining part.

5. Eventually you are saying you are still standing with your veto even
though I think I've clearly reasoned it and reverted the part you weren't
agreeing with (see 4).

I can't get it. What exactly am I doing wrong? Just because you don't
like it?

Again, I respect vetoes and, of course, your opinion. I just want our
developing process and communication to be... like more straight-forward,
you know. I would like the new features to be more appreciated and
discussed, rather than be completely rejected due to minor and fixable
mistakes. It's fine to make mistakes. And those should be discussed and
fixed without blaming either authors and their opinions or anything else
like this. This is my understanding of the commit-then-review policy that
we use in svn.

ps: I've drafted a patch that adds the same alias as for those two (default
and current locale) for utf8 as well, which would be also really useful for
utf8-cmdline-prototype, since I had to specify "UTF-8" constant for
multiple times. Initially I wanted to discuss it in a separate thread, but
I don't think it's worthed at this moment, so I'll just attach it
here, just to have it.

-- 
Timofei Zhakov
Index: subversion/include/svn_utf.h
===================================================================
--- subversion/include/svn_utf.h        (revision 1926352)
+++ subversion/include/svn_utf.h        (working copy)
@@ -54,7 +54,12 @@
  */
 #define SVN_APR_LOCALE_CHARSET (const char *)1
 
-  /**
+/**
+ * Indicate UTF8 charset
+ */
+#define SVN_UTF8_CHARSET (const char *)2
+
+/**
  * Initialize the UTF-8 encoding/decoding routines.
  * Allocate cached translation handles in a subpool of @a pool.
  *
Index: subversion/libsvn_repos/fs-wrap.c
===================================================================
--- subversion/libsvn_repos/fs-wrap.c   (revision 1926344)
+++ subversion/libsvn_repos/fs-wrap.c   (working copy)
@@ -32,6 +32,7 @@
 #include "svn_props.h"
 #include "svn_repos.h"
 #include "svn_time.h"
+#include "svn_utf.h"
 #include "svn_sorts.h"
 #include "svn_subst.h"
 #include "repos.h"
@@ -276,7 +277,7 @@
       svn_string_t *new_value;
 
       SVN_ERR(svn_subst_translate_string2(&new_value, NULL, normalized_p,
-                                          value, "UTF-8", TRUE,
+                                          value, SVN_UTF8_CHARSET, TRUE,
                                           result_pool, scratch_pool));
       *result_p = new_value;
     }
Index: subversion/libsvn_subr/subst.c
===================================================================
--- subversion/libsvn_subr/subst.c      (revision 1926344)
+++ subversion/libsvn_subr/subst.c      (working copy)
@@ -1973,7 +1973,7 @@
       return SVN_NO_ERROR;
     }
 
-  if (encoding && !strcmp(encoding, "UTF-8"))
+  if (encoding && (encoding == SVN_UTF8_CHARSET || !strcmp(encoding, "UTF-8")))
     {
       val_utf8 = value->data;
     }
Index: subversion/libsvn_subr/utf.c
===================================================================
--- subversion/libsvn_subr/utf.c        (revision 1926344)
+++ subversion/libsvn_subr/utf.c        (working copy)
@@ -171,11 +171,15 @@
     frompage = "APR_LOCALE_CHARSET";
   else if (frompage == SVN_APR_DEFAULT_CHARSET)
     frompage = "APR_DEFAULT_CHARSET";
+  else if (frompage == SVN_UTF8_CHARSET)
+    frompage = SVN_APR_UTF8_CHARSET;
 
   if (topage == SVN_APR_LOCALE_CHARSET)
     topage = "APR_LOCALE_CHARSET";
   else if (topage == SVN_APR_DEFAULT_CHARSET)
     topage = "APR_DEFAULT_CHARSET";
+  else if (topage == SVN_UTF8_CHARSET)
+    topage = SVN_APR_UTF8_CHARSET;
 
   return apr_pstrcat(pool, "svn-utf-", frompage, "to", topage,
                      "-xlate-handle", SVN_VA_NULL);
@@ -206,6 +210,8 @@
     return APR_DEFAULT_CHARSET;
   else if (charset == SVN_APR_LOCALE_CHARSET)
     return APR_LOCALE_CHARSET;
+  else if (charset == SVN_UTF8_CHARSET)
+    return SVN_APR_UTF8_CHARSET;
   else
     return charset;
 }
@@ -263,7 +269,9 @@
       else
         errstr = apr_psprintf(pool,
                               _("Can't create a character converter from "
-                                "'%s' to '%s'"), frompage, topage);
+                                "'%s' to '%s'"),
+                              get_apr_xlate_charset(frompage),
+                              get_apr_xlate_charset(topage));
 
       /* Just put the error on the stack, since svn_error_create duplicates it
          later.  APR_STRERR will be in the local encoding, not in UTF-8, 
though.
Index: subversion/libsvn_subr/win32_xlate.c
===================================================================
--- subversion/libsvn_subr/win32_xlate.c        (revision 1926344)
+++ subversion/libsvn_subr/win32_xlate.c        (working copy)
@@ -102,7 +102,7 @@
       *page_id_p = CP_THREAD_ACP; /* Valid on Windows 2000+ */
       return APR_SUCCESS;
     }
-  else if (!strcmp(page_name, "UTF-8"))
+  else if (page_name == SVN_UTF8_CHARSET || !strcmp(page_name, "UTF-8"))
     {
       *page_id_p = CP_UTF8;
       return APR_SUCCESS;
Index: subversion/svnmucc/svnmucc.c
===================================================================
--- subversion/svnmucc/svnmucc.c        (revision 1926344)
+++ subversion/svnmucc/svnmucc.c        (working copy)
@@ -463,7 +463,7 @@
       svn_string_t *message = svn_string_create(lmb->log_message, pool);
 
       SVN_ERR_W(svn_subst_translate_string2(&message, NULL, NULL,
-                                            message, "UTF-8", FALSE,
+                                            message, SVN_UTF8_CHARSET, FALSE,
                                             pool, pool),
                 _("Error normalizing log message to internal format"));
 
@@ -884,7 +884,7 @@
               svn_string_t *translated_value;
               SVN_ERR_W(svn_subst_translate_string2(
                             &translated_value, NULL, NULL, action->prop_value,
-                            "UTF-8", FALSE, pool, pool),
+                            SVN_UTF8_CHARSET, FALSE, pool, pool),
                         "Error normalizing property value");
               action->prop_value = translated_value;
             }
Index: subversion/svnsync/sync.c
===================================================================
--- subversion/svnsync/sync.c   (revision 1926344)
+++ subversion/svnsync/sync.c   (working copy)
@@ -74,7 +74,7 @@
   SVN_ERR_ASSERT((*str)->data != NULL);
 
   if (source_prop_encoding == NULL)
-    source_prop_encoding = "UTF-8";
+    source_prop_encoding = SVN_UTF8_CHARSET;
 
   new_str = NULL;
   SVN_ERR(svn_subst_translate_string2(&new_str, NULL, was_normalized,

Reply via email to