On Tue, 2010-05-04 at 17:55 +0200, Hyrum K. Wright wrote: > On Tue, May 4, 2010 at 12:45 PM, Stefan Sperling <s...@elego.de> wrote: > > > On Tue, May 04, 2010 at 09:45:44AM -0000, hwri...@apache.org wrote: > > > Author: hwright > > > Date: Tue May 4 09:45:43 2010 > > > New Revision: 940786 > > > > > > URL: http://svn.apache.org/viewvc?rev=940786&view=rev > > > Log: > > > Add a callback to the public patch API, to allow consumers to collect > > > information about the patch targets. This replaces the reject_tempfiles > > > and patched_tempfiles return parameters. > > > > > > * subversion/tests/libsvn_client/client-test.c > > > (patch_collection_baton, patch_collection_func): New. > > > (test_patch): Use the baton to collect the tested information. > > > > > > * subversion/svn/patch-cmd.c > > > (svn_cl__patch): Remove the tempfiles, and don't implement a patch > > callback. > > > > > > * subversion/include/svn_client.h > > > (svn_client_patch_func_t): New. > > > (svn_client_patch): Remove the output hashes, and add a callback and > > baton. > > > > > > * subversion/libsvn_client/patch.c > > > (init_patch_target): Pass through the REMOVE_TEMPFILES param. > > > (apply_one_patch): Adjust parameters, and call the callback, where > > > appropriate. > > > (apply_patches_baton_t): Adjust members to refer to the updated > > parameters. > > > (apply_patches): Pass through parameters to apply_one_patch(). > > > (svn_client_patch): Set the updated baton parameters. > > > > > > Modified: > > > subversion/trunk/subversion/include/svn_client.h > > > subversion/trunk/subversion/libsvn_client/patch.c > > > subversion/trunk/subversion/svn/patch-cmd.c > > > subversion/trunk/subversion/tests/libsvn_client/client-test.c > > > > > > Modified: subversion/trunk/subversion/include/svn_client.h > > > URL: > > http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_client.h?rev=940786&r1=940785&r2=940786&view=diff > > > > > ============================================================================== > > > --- subversion/trunk/subversion/include/svn_client.h (original) > > > +++ subversion/trunk/subversion/include/svn_client.h Tue May 4 09:45:43 > > 2010 > > > @@ -4833,6 +4833,25 @@ svn_client_info(const char *path_or_url, > > > */ > > > > > > /** > > > + * The callback invoked by svn_client_patch(). For each patch target, > > > + * call this function for @a local_abspath, and return the @a > > patch_abspath > > > + * and @a reject_abspath. Neither @a patch_abspath or @a reject_abspath > > are > > > + * guaranteed to exist (depending on the @a remove_tempfiles parameter > > for > > > + * svn_client_patch() ). > > > > > > What is the reason for the remove_tempfiles parameter? > > Do you envision the callback to be used for additional tasks in the future? > > Why not just drop the remove_tempfiles parameter and clean up the > > tempfiles if the caller passes NULL for the callback? > > > I feel uncomfortable overloading the callback parameter in that way. The > value of the function pointer should not have operational side effects, such > as leaving or not leaving tempfiles in the working copy. It makes more > sense to me to make that explicit. If we currently require that > remove_tempfiles should be FALSE when no callback is given, we can document > it as such, and add an assertion in the function.
+1 on not overloading it. That ties in with my queries on the callback doc string: part of the reason I was asking for lots of clarification is because I got the feeling that something "magic" was happening that wasn't being said - such as leaving temp files iff the callback function pointer was non-null - but wasn't sure whether that was supposed to be implied or not. - Julian
