On Sun, Oct 1, 2023 at 9:58 AM Daniel Sahlberg <daniel.l.sahlb...@gmail.com> wrote: > > Hi, > > Issue SVN-1778 [1] is about the Python bindings, class FileDiff, where get_pipe() called an external program diff (under Windows: diff.exe). Since diff.exe has to be installed (and added to PATH) manually on Windows the function would fail. The same is true on other os:es but diff is probably more commonly present by default. > > r1824410 added support for running the internal Subversion diff functions (svn.diff.file_diff_2 and svn.diff.file_output_unified4) as long as the argument diffoptions is None (the default). Since diffoptions is passed as arguments to the diff command, it seems reasonable to require the presence of diff (or diff.exe) if one uses diffoptions. > > I'm suggesting to close this issue - it is not completely solved but when using diffoptions,it is probably expected that this arguments is passed on to whatever diff/diff.exe executable is present on the system and thus it is not unreasonable to require the end user to install diff.exe > > Kind regards, > > Daniel Sahlberg > > [1] https://issues.apache.org/jira/browse/SVN-1778
This (somewhat platform-specific) "gotcha" isn't completely obvious from a first glance at the code. Your explanation above makes the failure mode and its reasons more clear. So, to close SVN-1778, I would suggest only to add some documentation of the above fact to the function. I'll be happy to compose a suggested docstring. I don't know how it fails (with a cryptic traceback message?) but if we want to get fancy, perhaps a failure could give some user-friendly hint about things to check for (such as whether a diff tool is available and diffoptions are correct). I noticed that there aren't docstrings on most of the functions in subversion/bindings/swig/python/svn/fs.py, and I don't know whether that's deliberate. If it is, then the suggested docstring could be a comment instead. Cheers Nathan
