On 12.01.2012 20:07, Philip Martin wrote:
> Stefan Küng<tortoisesvn_at_gmail.com> writes:
>> Could someone please review my patch? I'm sure you guys can see
>> immediately whether this is save or not. If it's ok, then I'll commit
>> it later.
> Index: subversion/libsvn_client/patch.c
> --- subversion/libsvn_client/patch.c (Revision 1230694)
> +++ subversion/libsvn_client/patch.c (Arbeitskopie)
> @@ -2920,8 +2920,9 @@
> while (patch);
> - /* Delete directories which are empty after patching, if any. */
> - SVN_ERR(delete_empty_dirs(targets_info, ctx, dry_run, scratch_pool));
> + if (!dry_run)
> + /* Delete directories which are empty after patching, if any. */
> + SVN_ERR(delete_empty_dirs(targets_info, ctx, dry_run, scratch_pool));
> SVN_ERR(svn_diff_close_patch_file(patch_file, iterpool));
> It doesn't look right to me. Use dry_run to in two places, to determine
> whether to call delete_empty_dirs and as a parameter to
> delete_empty_dirs isn't sensible.
Question is: do we need the notifications for deleting empty dirs in a
dry-run? If yes, then this get's complicated:
the error is thrown from svn_wc_walk_status() called in check_dir_empty.
I could just not call that function in case of a dry-run, or catch that
specific error and go on. But then the notifications wouldn't be
completely accurate anymore.
So assuming we don't want to extend svn_wc_walk_status to take a dry-run
param, the notifications for deleting empty dirs wouldn't be accurate.
Should I just change delete_empty_dirs() to not use the dry_run param
and not get the notifications for those in a dry-run?
I think no notifications is better than inaccurate ones (at least in a
oo // \\ "De Chelonian Mobile"
(_,\/ \_/ \ TortoiseSVN
\ \_/_\_/> The coolest Interface to (Sub)Version Control
/_/ \_\ http://tortoisesvn.net
Received on 2012-01-12 20:19:47 CET