Hyrum,
this is going to sound inconsiderate after I've just reviewed the
implementation, but I've just been thinking back to why we're doing
this, and I am still not convinced that this is not over-generalising.
At the UI level, I have no evidence that we need a general
"--ignore-prop" feature, and, although we could commit this for
experimentation, I would not like to see it in a released version
without knowing it's valuable enough to justify its existence. (If it is
valuable, then it seems likely that the converse feature for noticing
ONLY certain properties would be needed too.)
At the libsvn_client level, exactly the same applies.
Do you need to commit this in order to continue developing the idea?
Might it be better to work on a branch, if this is still in the
proof-of-concept stage?
A few more comments on the current implementation.
On Wed, 2008-09-10 at 13:01 -0500, Hyrum K. Wright wrote:
[...]
> Yes, I do plan on making it work with 'status -u'. I think that lots of the
> machinery in this patch can be reused in that case (it may already work!), but I
> haven't yet thoroughly tested it.
That's good.
[...]
> >> + case opt_ignore_prop:
> >> + if (opt_state.ignored_props == NULL)
> >> + opt_state.ignored_props = apr_hash_make(pool);
> >> +
> >> + apr_hash_set(opt_state.ignored_props, apr_pstrdup(pool,
> opt_arg),
> >> + APR_HASH_KEY_STRING, (void *) 0xdeadbeef);
> >
> > Cute, but the names of all cows must be declared in a header file.
> Just
> > use NULL :-)
>
> My initial reason for doing this was do that if I start seeing the
> value 0xdeadbeef somewhere, I'd know from whence it was coming. But I
> suppose that isn't really needed here, so I've gone with the somewhat
> duller NULL. :)
Heh. It doesn't work at all now! Sorry about that. I forgot that null
values are not allowed; it means "delete this key".
[...]
> [[[
> Add support for ignoring arbitrary property modifications in 'svn st'.
> This currently only works locally, not with 'svn st -u'.
>
> * subversion/include/svn_wc.h
> (svn_wc_props_modified2): New.
Can you see if you can use the existing svn_wc_get_prop_diffs() instead?
I think there is no need for this new function. (If you do replace
svn_wc_props_modified_p() with svn_wc_props_modified2(), I would have
some comments on that.)
> Index: subversion/svn/main.c
> ===================================================================
> --- subversion/svn/main.c (revision 33015)
> +++ subversion/svn/main.c (working copy)
> @@ -269,6 +270,8 @@
> "('merged', 'eligible')")},
> {"reintegrate", opt_reintegrate, 0,
> N_("lump-merge all of source URL's unmerged changes")},
> + {"ignore-prop", opt_ignore_prop, 1,
> + N_("Ingore changes to property ARG")},
Use lower-case "i", and spelling "ignore".
Thanks for fixing the other bits.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-09-11 12:30:38 CEST