On Sun, Apr 5, 2026 at 4:43 PM Branko Čibej <[email protected]> wrote: > > On 5. 4. 26 16:33, Timofei Zhakov wrote: > > On Sun, Apr 5, 2026 at 4:07 PM Branko Čibej <[email protected]> wrote: > > On 5. 4. 26 11:44, [email protected] wrote: > > Author: rinrab > Date: Sun Apr 5 09:44:54 2026 > New Revision: 1932855 > > Log: > Export a function for parsing a single opt_revision into the public API. We'll > probably use it in svnbrowse. Neither of other tools need it because they > primarly operate over a revision range for which there is a separate function > in the API. > > > Is making this public really necessary? A wrapper for > svn_opt_parse_revision() that checks that there was only one revision in the > argument is literally two lines of code that can be local in svnbrowse. > > Not exactly. It wraps parse_one_rev(). > > > I know this implementation does, but parse_one_rev() is static. > > > svn_opt_parse_revision() has extra code for revision ranges. It > technically is possible to use it and just check that the end is > missing (which svn does if revision ranges are not supported for the > specific subcommand), but I believe it's easier to have more specific > APIs. > > > Every new public API is another that needs to be maintained forever, that's > all I'm trying to say. I really don't think it's worth it for one usage in > our code.
That is true. But it's also wrong to write extra workarounds if there is this option to just add a more convenient API. It's not always the case. It makes sense only if the API *makes sense*. Here I think it does. It's the svn_opt_parse_revision() which is built on parse_one_rev() primitive. At the end of the day, svn_opt.h exists to simplify our lives by providing everything one might need to implement a command line parser. svn can parse -r syntax without opt. It just makes it simpler and obvious for readers. -- Timofei Zhakov
