Re: started applying Marcus' patch

Karl, I've now finished reviewing the UTF-8 checkings (including all
followups to this date). These are the issues I found:

*) First of all, the property value encoding thing. There seems to be
   an intention that properties whose names begin with "svn:" should
   have their values transcoded, and other properties not. I have two
   things to say about this:

   o) This convention is not followed by neither diff (which will
      print all propery values without transcoding) nor propedit
      (which uses svn_cl__edit_externally(), which transcodes
       everything). Should be fixed before alpha.

   o) This very ad-hoc convention is fine for alpha, but I trust it's
      not intended as the final solution? It has two problems:

      -) It is unintuitive. It is not obvious to the user why
         only certain properties are magical in this way based on
         their name.

      -) It is too limiting. Why should transcoding only be
         available to svn: properties? It is useful for user
         properties as well, so there should be a way to use it.

*) The --message-encoding option affects decoding of argument to -m
   and not only messages taken from -F. If this behaviour is correct,
   the help message should change to reflect it.

*) In subversion/libsvn_client/diff.c, the return values of
   svn_io_file_printf are ignored.

*) The FIXME in svn_cl__args_to_target_array could be taken care of
   now, since the function has been modified to return svn_error_t*.
   (Actually, I thought that was the entire purpose of the prototype
    change, but that FIXME is still there...)

*) Did you really intend to remove the comment

     /* Open the file to be used as the base for second revision */

   in apply_textdelta in subversion/libsvn_client/repos_diff.c? That
   looked like a mistake.

*) And finally, I have to ask about the change to check_non_ascii.
   Not about the && thing, that was obviously just a typo. But what
   made you decide that a bit test was better than the arithmetic
   comparison? Readability? Performance? Portability?

   In fact, the bit test is a teeny bit less portable, because if you
   (against all reasonable probability) have 9-bit chars for example,
   the test will miss characters in the range 256-383. While this is
   technically how the comment says it should work, it's probably not
   what we want.

  // Marcus

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Jul 18 04:45:19 2002