Marcus Comstedt <marcus@mc.pp.se> writes:
> Karl, I've now finished reviewing the UTF-8 checkings (including all
> followups to this date). These are the issues I found:
Wow, thanks!
I'll be going over these in the code tomorrow...
> *) 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.
Yes, the property transcoding handling needs some more work to be
totally consistent.
> 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.
I'm not sure what to do about the general problem, except to document
around it. Perhaps we can find a nicer solution after Alpha. Say, a
configuration option that names which properties get transcoded, and
the default just happens to include the requisite "svn:" properties.
There may be other answers, though.
We won't get all of this sorted out for Alpha, and that's okay.
> *) 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.
I _think_ it is the behavior which should change, will talk it over
with Ben tomorrow and see if there's a rationale behind the current
way first.
> *) In subversion/libsvn_client/diff.c, the return values of
> svn_io_file_printf are ignored.
Thanks, will fix.
> *) 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...)
Thanks again.
> *) 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.
Okay, will look.
> *) 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.
It was for performance only. I never knew of a portability issue. Is
it really an issue anywhere? I thought char was guaranteed to be an
eight-bit value in ANSI C, but perhaps I'm confusing practice with
promise...
Thanks again,
-K
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Jul 18 07:15:14 2002