Re: [PATCH] Fix python bindings for svn_client_info()

Jelmer Vernooij wrote:
> The attached patch fixes the use of the svn.client.info() function in
> the Python bindings. Patch against trunk.

I offer only a super-quick drive-by review.

> [[[
> * subversion/bindings/swig/svn_client.i: Add typemap for svn_info_receiver_t in Python
>
> * subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c
> (svn_swig_py_info_receiver): Add wrapper function for
> svn_info_receiver_t that calls a Python function
>
> * subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c:
> Add prototype for svn_info_receiver_t
> ]]]

Nice, clean log message. Lacks a brief summary (such as "Add support for
svn_client_info() to the Python bindings") and an 80-column fit (solution is
something like s/svn_client.i: /svn_client.i\n /)

> Index: subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c
> ===================================================================
> --- subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c (revision 19389)
> +++ subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c (working copy)
> @@ -1814,7 +1815,53 @@
> return err;
> }
>
> +svn_error_t *svn_swig_py_info_receiver(void *baton,
> + const char *path,
> + const svn_info_t *info,
> + apr_pool_t *pool)

Ick. Tabstobs. Badness. (This problem is already present in the new
header file prototype.)

> +{
> + PyObject *receiver = baton;
> + PyObject *result, *py_pool, *py_info;
> + svn_error_t *err = SVN_NO_ERROR;
> +
> + if ((receiver == NULL) || (receiver == Py_None))
> + return SVN_NO_ERROR;
>
> + svn_swig_py_acquire_py_lock();
> +
> + py_pool = make_ob_pool(pool);
> + if (py_pool == NULL) {
> + err = callback_exception_error();
> + goto finished;
> + }
> +
> + py_info = svn_swig_NewPointerObjString(info, "svn_info_t *", py_pool);
> + if (py_info == NULL) {
> + Py_DECREF(py_pool);
> + err = callback_exception_error();
> + goto finished;
> + }
> +
> + if ((result = PyObject_CallFunction(receiver,
> + (char *)"sOO",
> + path, py_info,
> + py_pool)) == NULL)
> + {
> + err = callback_exception_error();
> + }

I expected this implementation to be more like the support for
svn_client_status. So, add:

   DECLARE_SWIG_CONSTRUCTOR(info, svn_info_dup)

And then instead of all the stuff above, just do:

  if ((result = PyObject_CallFunction(receiver,
                                      (char *)"sO&",
                                      path, make_ob_info,
                                      info)) == NULL)
    {
      err = callback_exception_error();
    }

-- 
C. Michael Pilato <cmpilato@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand