[RFC] Introducing svn_client_make_diff_args? (Was: RE: svn commit: r33116 - in branches/ignore-mergeinfo/subversion: include libsvn_client svn)

> @@ -2225,8 +2228,36 @@ svn_client_blame(const char *path_or_url
> * @note @a relative_to_dir doesn't affect the path index generated by
> * external diff programs.
> *
> + * @since New in 1.6.
> + */
> +svn_error_t *
> +svn_client_diff5(const apr_array_header_t *diff_options,
> + const char *path1,
> + const svn_opt_revision_t *revision1,
> + const char *path2,
> + const svn_opt_revision_t *revision2,
> + const char *relative_to_dir,
> + svn_depth_t depth,
> + svn_boolean_t ignore_ancestry,
> + svn_boolean_t no_diff_deleted,
> + svn_boolean_t ignore_content_type,
> + svn_boolean_t ignore_mergeinfo,
> + const char *header_encoding,
> + apr_file_t *outfile,
> + apr_file_t *errfile,
> + const apr_array_header_t *changelists,
> + svn_client_ctx_t *ctx,
> + apr_pool_t *pool);

This method has 5 different versions over 7 subversion point releases, each adding just on or a few 'optional' parameters.

Shouldn't we create some kind of context/args object that we can extend in future versions for methods that probably have yet another version in the next release?

Adding boolean number 4 to a function doesn't make user code more understandable, and with a context/args struct with a constructor function we can just extend that structure in the next release without breaking the ABI.

We do use argument structures in a few places in the subversion code (think user context, status, authorization, ...), but I think diff would be a good candidate to follow this construction. And there are probably a few other places that could benefit from such argument structures.

=====================
In SharpSvn I use the convention of args objects for most optional parameters, so I don't have to create another override for every future version of subversion that adds just another boolean parameter like this.

Using the old NSvn that followed the subversion arguments like most other bindings, you would use something like:
[[
Client.Diff("a", "b", diffStream, true, true, false, false, true);
]]
Completely unreadable without intellisense or documentation open.

While a SharpSvn user would write something like:

[[
SvnDiffArgs args = new SvnDiffArgs();
args.IgnoreContentType = true;
args.IgnoreMergeInfo = true;

Client.Diff("a", "b", diffStream, args);
]]

A little bit more code, but no need to set default values. I think everybody can understand it without looking at the documentation.

(Using C# 3.0 instead of 2.0 would allow you to use a more compact syntax)

Adding a new property to the args object does not break the ABI and does not require a new compatibility wrapper to maintain forever.

        Bert

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-09-17 12:47:30 CEST