"Rui, Guo" <timmyguo_at_mail.ustc.edu.cn> writes:
>> We usually give the full path (from source tree root) of each file.
>> It's not a big deal, just fix up the log message when you get a chance.
>
> Well, I just followed the hacking guide.
Indeed -- I never noticed that recommendation! Sorry. It's fine either
way, then. I have to admit I prefer having the full path, since we
often have files with the same name (e.g., "lock.c") in different places
in the tree, and if the full path is given I don't have to glance
backwards to see which file we're dealing with.
>> > @@ -2558,8 +2560,19 @@ svn_wc_remove_from_revision_control(svn_
>> > parent_dir, pool));
>> > SVN_ERR(svn_wc_entries_read(&entries, parent_access, TRUE,
>> > pool));
>> > - svn_wc__entry_remove(entries, base_name);
>> > - SVN_ERR(svn_wc__entries_write(entries, parent_access, pool));
>> > +
>> > + /* An exception: When the path is at svn_depth_exclude,
>> > + the entry in the parent directory should be preserved
>> > + for bookkeeping purpose. This only happens when the
>> > + function is called by svn_wc_crop_tree(). */
>> > + dir_entry = apr_hash_get(entries, base_name,
>> > + APR_HASH_KEY_STRING);
>> > + if (dir_entry->depth != svn_depth_exclude)
>> > + {
>> > + svn_wc__entry_remove(entries, base_name);
>> > + SVN_ERR(svn_wc__entries_write(entries, parent_access, pool));
>> > +
>> > + }
>> > }
>> > }
>>
>> I realize it's not your code, but I find the reuse of "entries"
>> confusing here, and wish there were a separate, block-local variable
>> named "parent_entries" or something. Do you agree?
>
> Yep! Done, waiting for commit.
Oh -- please do that on trunk (+1 from me), and just have it come over
to the branch the next time you merge.
> In fact, I can also simply call svn_wc_remove_from_revision_control()
> for excluded subdirectory (even though my code may be a bit faster?)
> -- the document of svn_wc_remove_from_revision_control() seems a bit
> misleading. If I understand correctly, it can handle any target rather
> than just 'file and this_dir'.
The documentation claims it can only do file and this_dir, but I'm not
sure why it has that requirement...
>> > }
>> > - /* XXX: What about svn_node_none & svn_node_unkown? Currently assume
>> > - svn_node_dir*/
>> > + else
>> > + {
>> > + /* XXX: What about svn_node_none & svn_node_unkown? Currently assume
>> > + svn_node_dir*/
>> > + }
>>
>> As always, the second sentence of that comment seems inaccurate to me :-).
>
> Shy. It dates back to the time that I created the file. I just have no idea if
> I need to shrow an exception here.
I think you should. It's better to be strict at first; we can decide to
loosen up later if it's a problem.
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-06-15 16:31:19 CEST