Back in the dark ages, before we had nailed down svn_path, we determined
that the structure of working copies demands that all operations be
rooted in a directory, even if the operation applies to a file. So, we
have this function svn_wc_get_actual_target, which splits a path into an
"anchor" and a "target". Examples:
svn update foodir/barfile --> "foodir"/"barfile"
svn update foodir --> "foodir"/NULL
svn update --> ""/NULL
This discipline is also reflected in the tree editor (you can only
open_root a directory, not a file), since tree edits are often meant to
be applied to working copies. Because of that, svn_repos_dir_delta()
accepts separate src_parent_dir (read: anchor) and src_entry (read:
target) parameters. The four wc-updating ra_lib functions (do_update,
do_switch, do_status, do_diff) also accept a target parameter which is
expected to be NULL for operations on directories.
In issue 999, I wrote that target should probably be "", not NULL, for
directory operations. In many other contexts we use the empty path,
rather than NULL, to mean "right here". I have composed a patch
(attached) to make this change, and am looking for comments. There is
no way I will apply this change to the trunk if there isn't agreement
that it should be in 1.0, since the trunk should not start out life with
a subtle and permanent API change from the 1.0 branch.
Specifically, I'm looking for comments like:
* You think this is wrong, that we really should be using NULL for
the target, because this is different from other situations
where we use a path element of "right here."
* You think this is right (or at least debatable), but you would
rather live with this API wart for a very long time than apply
such a potentially destabilizing patch to the 1.0 branch.
* You think this is right, and should make it into 1.0.
As an aside, I am developing a distinct antipathy towards
svn_path_is_empty(), because it is verbose and confusing (confusing
because it's a test for a negative, rather than for a positive). The
attached patch uses it all over the place, because I didn't feel that it
was right to ignore an svn_path abstraction, but I would much rather
write "if (*target)" than "if (! svn_path_is_empty (target))". I'd like
to solicit opinions on that point as well. Should I nuke it from
orbit? (We use it in 21 places right now.) Should I leave it alone but
feel free to ignore it? Or should I grit my teeth and use it?
For consistency, use "" instead of NULL for a no-op target.
* libsvn_wc/update_editor.c, include/svn_wc.h (svn_wc_get_actual_target):
Set target to empty (not NULL) when anchor is subject.
* include/svn_repos.h (svn_repos_begin_report, svn_repos_dir_delta),
include/svn_wc.h (svn_wc_get_status_editor, svn_wc_get_update_editor,
include/svn_ra.h (do_update, do_switch, do_status, do_diff):
Expect target to be empty (not NULL) when anchor is subject.
* libsvn_wc/diff.c (directory_elements_diff),
libsvn_wc/status.c (delete_entry, close_directory):
libsvn_wc/update_editor.c (make_dir_baton, complete_directory,
libsvn_client/diff.c (diff_wc_wc, diff_repos_wc),
libsvn_client/commit.c (adjust_rel_targets, svn_client_commit),
libsvn_ra_dav/fetch.c (end_element, make_reporter):
Test for emptiness of target, rather than nullity.
* libsvn_client/switch.c (svn_client_switch),
libsvn_client/diff.c (do_merge, diff_repos_repos, diff_repos_wc),
Pass empty target (not NULL) when anchor is subject.
* libsvn_wc/diff.c (svn_wc_diff),
Simplify code by assuming that target is not NULL.
* libsvn_ra_svn/client.c (ra_svn_update, ra_svn_switch, ra_svn_status,
svnserve/serve.c (update, switch_cmd, status, diff):
Stop translating NULL to "" and back over the wire.
Received on Sun Dec 14 20:37:38 2003
To unsubscribe, e-mail: firstname.lastname@example.org
For additional commands, e-mail: email@example.com