Julian Foad <julianfoad_at_btopenworld.com> writes:
> Nothing heard. Committed revision 32483.
Sorry -- catching up after r32483:
The doc string now reads like this:
/* Get revision REV of the file that is at the root URL of RA_SESSION.
Store the file's text content in a new temporary file in the same
directory as the path WC_TARGET. Set *FILENAME to the path to that file.
Set *PROPS to a hash containing the file's properties.
All allocation occurs in POOL. */
/* ### TODO: Create the temporary file under .svn/tmp/ instead of next to
the working file. */
static svn_error_t *
single_file_merge_get_file(const char **filename,
svn_ra_session_t *ra_session,
apr_hash_t **props,
svn_revnum_t rev,
const char *wc_target,
apr_pool_t *pool)
One thing that's not clear is what "the file that is at the root URL of
RA_SESSION" means. Often, the root URL of an RA_SESSION is a directory,
and there are many files that could be in its root; here there is no
clear way here to know which of those files the doc string is talking
about. If this function requires that RA_SESSION be rooted at a file,
then it should say so, and ideally say what error would be returned if
RA_SESSION is rooted at a directory.
As far as phrasing goes: I find that sometimes it's conceptually clearer
to start from the result effects and work backwards. E.g.:
/* Set *FILENAME to the path of (and *PROPS to a hash containing the
properties of) the file at revision REV in the root url of
RA_SESSION. Store the file's text content in a new temporary file
in the same directory as the path WC_TARGET. [todo: need to
specify whether WC_TARGET can be a directory or not]
Do all allocation in POOL. */
Thoughts?
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-08-14 22:31:45 CEST