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

Re: started applying Marcus' patch

From: Karl Fogel <kfogel_at_newton.ch.collab.net>
Date: 2002-07-18 07:03:01 CEST

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

Thanks again,

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

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.