On Mon, Jul 5, 2010 at 10:26 AM, Julian Foad <julian.f...@wandisco.com> wrote:
> On Mon, 2010-07-05, Hyrum K. Wright wrote:
>> Julian's r960620 prompted me to do a bit of digging in the client
>> library. It turns out there is nowhere we use
>> svn_wc_translated_stream3() and svn_wc_translated_file2(), which are
>
> uh, svn_wc_translated_stream2() and svn_wc_translated_file3() ...
heh. :)
>> used to allow the client library to grab the keywords- and
>> eol-style-translated versions of a file. Are these APIs now obsolete?
>> Can they be removed (the most recent versions are new in 1.7)? Do
>> other API consumers depend on them?
>
> This doesn't impact directly on whether we should remove the latest
> (unused) versions and deprecate the previous versions of the APIs you
> mentioned, but ...
>
> I have been thinking the set of "translation" API functions is a bit
> ragged. I would like to see a smaller set of API functions with more
> distinct tasks, something like just the following pair:
>
> (A) a libsvn_subr API that translates a given stream according to a
> given set of properties, not implicitly referring to a versioned node.
>
> (B) a libsvn_wc API that grabs, from a versioned file, the set of
> properties necessary for translating it (note that this includes at
> least the rev, author, and date as well as the versioned properties
> svn:eol-style, svn:mime-type and svn:special);
>
> The purpose is to be able to translate arbitrary stored texts -
> especially conflict left/right/mine files stored in the pristine store -
> to and from Repository Normal Form without referring to a current
> versioned node at the time of translation.
Yep, reducing the coupling here would be good. I also ran across this
little gem in translate.c:
/* ### ugh. the repair behavior does NOT match the docstring. bleah.
### all of these translation functions are crap and should go
### away anyways. we'll just deprecate most of the functions and
### properly document the survivors */
Apparently the author was thinking along the same lines as you are now. :)
Another API anomaly I noticed was that translate_file() and
translate_stream() have two very different implementations. In my
mind, translate file would just wrap translate stream, but it appears
there is more magic going on here that meets the eye.
>> Relatedly, the one potential use for them is to replace the
>> libsvn_client call to svn_wc__versioned_file_modcheck() in merge.c.
>> But it feels there is a better solution for that one-off case.
>
> +1 to replacing that private call with some suitable public call. I
> expect to see several other places like this appear in the course of
> tree conflicts work.
Yeah. I think we should initially keep this API private, but at least
change the name to something a bit more descriptive of what it's
actually doing. Maybe svn_wc__normal_forms_match() or something.
-Hyrum
