Jon Trowbridge <trow@ximian.com> writes:
> * libsvn_wc/status.c:
> (assemble_status): Added a new argument, is_ignored. If it is set,
> files without entries that are present on the disk are given a
> text_status of svn_wc_status_none rather than
> svn_wc_status_unversioned.
> (add_status_structure): Added an is_ignored argument which is
> propogated to our call to assemble_status.
> (add_unversioned_items): Added an is_ignored argument, which is used
> to determine if we report ignored items.
> (svn_wc_status): In our call to assemble_status, pass in FALSE for the
> is_ignored parameter.
> (svn_wc_statuses): When calling add_status_structure, pass in FALSE
> for the is_ignored parameter.
Thanks for the repost!
Minor nit: would be nice to have a short introductory bit in the log
message, summarizing the change and explaining the motivation. (I
think you actually wrote such a paragraph in an email to Philip Martin
recently). Not a big deal, anyone can easily make up the necessary
text after some thought, just would be more convenient if you wrote
it.
> Index: status.c
Best to generate patches from the top of your tree, so we get a useful
relative path here. Ther are, like, ten zillion files named
"status.c" in Subversion :-). (Of course can figure it out from the
log message, so no real problem.)
> ===================================================================
> --- status.c
> +++ status.c Thu Aug 29 23:07:51 2002
> @@ -93,6 +93,10 @@
> If GET_ALL is zero, and ENTRY is not locally modified, then *STATUS
> will be set to NULL. If GET_ALL is non-zero, then *STATUS will be
> allocated and returned no matter what.
> +
> + If IS_IGNORED is set and this is a non-versioned entity, set the
> + text_status to svn_wc_status_none. Otherwise set the text_status
> + to svn_wc_status_unversioned.
> */
Again, very minor nit: for consistency with the rest of the doc
string, say "if IS_IGNORED is non-zero" or "if IS_IGNORED is true".
(Personally I don't mind the set/unset language in itself, though some
do, but in any case this doc string wasn't using it before now).
> /* Add all items that are NOT in ENTRIES (which is a list of PATH's
> versioned things) to the STATUSHASH as unversioned items,
> - allocating everything in POOL. If IGNORES is non-NULL, it contains
> - the default ignores, else this is an indication that no ignores
> - should be honored. */
> + allocating everything in POOL. IGNORES contains the list of
> + patterns to be ignored. If NO_IGNORE is set, all unversioned items
> + will be added; otherwise we will only add the items that do not match
> + any of the ignore patterns.
> +*/
(I'm probably missing something here, but...)
Why do we now have both an IGNORES list and a NO_IGNORE flag?
Especially given that the meaning of IGNORES == null was already
defined to mean what NO_IGNORE now means.
In other words, if a caller would pass NO_IGNORE after your patch, why
would they not simply pass NULL for IGNORES before your patch?
Oh wait, it must be because if NO_IGNORE is set, then we pass
ignore_me along to add_status_structure(), thus resulting in a
different svn_wc_status_foo value for that item. Is that it? But in
that case, you probably should mention this behavior (at least give
the specific svn_wc_status_foo values) in the doc string -- as it
stands, the documentation creates more questions than it answers :-).
Also, minor nit:
> + will be added; otherwise we will only add the items that do not match
> + any of the ignore patterns.
Instead of saying "any of the ignore patterns", say "any of the
patterns in IGNORES", so it's completely clear what's going on. If
indeed that *is* what's going on :-)...
-K
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Aug 30 08:16:52 2002