On Sun, Dec 16, 2018 at 11:28 AM Daniel Shahaf <d.s_at_daniel.shahaf.name>
> Troy Curtis Jr wrote on Sun, 16 Dec 2018 09:59 -0500:
> > But there was one item I wanted to talk about related to the patch. I
> > agree that "svn_stringbuf_t" => "bytes" and "svn_string_t" => "str" is
> > right general path,
> Are you sure about this? Property values can be binary data (so not
> representable as 'str') but are usually represented by «svn_string_t *»;
> for example, svn_repos_parse_fns3_t::set_node_property().
Ah yes, I suppose this also has bearing on the discussion in the other
to returning property values. With that in mind, perhaps your suggestion
defaulting to always being Bytes for char * , svn_string_t, and
svn_stringbuf_t unless there
is a specific circumstance not to makes the most sense as the general
Yasuhito, I suppose that means we should probably tweak the typemaps to
principle. Have you already started down that path based on the properties
> > but after the patch there is now a "svn_stringbuf_t
> > *output" typemap that is still using "PyStr" instead of "PyBytes".
> > Further, the only usage of "svn_stringbuf_t *output" is in
> > "svn_fs_print_modules" and "svn_ra_print_modules", where the function
> > parameter is actually an output parameter, so the appropriate typemap is
> > probably "argout" instead of "in".
> > Otherwise, the patch LGTM. Yasuhito, if you want to review my changes to
> > your patch and perhaps address the issue in the last paragraph then I can
> > commit your patch to swig-py3. I'll also start working more on the
> > branch to get it to the point that we can actually get this branch merged
> > into trunk finally.
> Great. :-)
Received on 2018-12-16 18:09:21 CET