[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn_client_status6() and svn_client_patch_func_t doc strings

From: Julian Foad <julianfoad_at_apache.org>
Date: Fri, 8 Dec 2017 09:49:16 +0000

Troy Curtis Jr wrote:
> Julian Foad wrote:
> [[[
> Index: subversion/include/svn_client.h
> ===================================================================
> --- subversion/include/svn_client.h     (revision 1817399)
> +++ subversion/include/svn_client.h     (working copy)
> @@ -2514,9 +2514,9 @@ typedef svn_error_t *(*svn_client_status
>   *      *result_rev is not meaningful unless @a check_out_of_date is
>   *      set).
>   *
> - *    - If @a check_working_copy is not set, do not scan the working
> - *      copy for local modifications. This parameter will be ignored
> - *      unless @a check_out_of_date is set.  When set, the status
> + *    - If @a check_working_copy is false, do not scan the working
> + *      copy for local modifications.  This parameter will be
> assumed true
> + *      unless @a check_out_of_date is set.  When false, the status
>   *      report will not contain any information about local changes in
>   *      the working copy; this includes local deletions and
>   *      replacements.
> @@ -7456,18 +7456,24 @@ svn_client_min_max_revisions(svn_revnum_
>   */
>
>  /**
> - * The callback invoked by svn_client_patch() before attempting to
> patch
> - * the target file at @a canon_path_from_patchfile (the path as
> parsed from
> - * the patch file, but in canonicalized form). The callback can set
> - * @a *filtered to @c TRUE to prevent the file from being patched,
> or else
> + * The callback invoked by svn_client_patch() when patching each
> target file.
> + *
> + * Called after putting the patch result and any reject in
> temporary files,
> + * before moving those files to the real location to complete the
> patching.
> + *
> + * The callback can set @a *filtered to @c TRUE to prevent moving the
> + * temporary files to the real location to complete the patching,
> or else
>   * must set it to @c FALSE.
>   *
> + * @a canon_path_from_patchfile is the path as parsed from the
> patch file,
> + * but in canonicalized form.
> + *
>
> Definitely much clearer language!
>
> - * Because the callback is invoked before the patching attempt is made,
> + * ### ? Because the callback is invoked before the patching
> attempt is made,
>   * there is no guarantee that the target file will actually be patched
>   * successfully. Client implementations must pay attention to
> notification
>   * feedback provided by svn_client_patch() to find out which paths
> were
> ]]]
>
>
> How about:
>
> The callback is invoked before the patching attempt is made and
> therefore there is
> no guarantee that the target file will actually be patched successfully.

Sorry, I didn't explain why I queried this paragraph. To me, "the
patching attempt" began before the callback, which is the main thing I
was clarifying in the first hunk above, so this paragraph is inaccurate.
It is not clear to me whether the client really needs to pay attention
to notifications, or if the callback can just look at whether any
non-empty "reject" file was created, to know whether there was a
problem. If there was no problem at this stage, then the patching is
more or less guaranteed to be successful -- there are very few reasons
why the final "move into place" step might fail.

- Julian
Received on 2017-12-08 10:49:22 CET

This is an archived mail posted to the Subversion Dev mailing list.