On Thu, 2008-08-28 at 14:04 -0400, Karl Fogel wrote:
> Julian Foad <julianfoad_at_btopenworld.com> writes:
> > I'm showing this UNFINISHED patch to ask: Does what I'm doing here look
> > like a sane improvement?
> >
> > [...]
> >
> > Provide factory functions for creating svn_wc_conflict_description_t objects,
> > because we require the user to use them. Make two different functions, one
> > for creating a text conflict description and one for creating a property
> > conflict description, because the required information differs considerably.
>
> Are you finding in practice that you want constructors, or are you
> anticipating the need? This route doesn't look insane, but I don't have
> enough experience to know if it's really necessary or not...
I'm finding that
(a) we implied in the doc-string that there should be a constructor so
that the structure can be extended in a backward-compatible way, but
there wasn't one;
(b) there are many rules about what fields make sense in various
combinations for different types of conflict, and those rules aren't
clearly documented and there is no place where they can be
programatically enforced or encouraged. Constructors provide a place
where the user can confidently gather all the required data in one
place. Specific constructors for different types of conflict enable the
user to ignore the fields that are not relevant.
(c) when it comes to extending it to hold tree conflicts as well, the
idea that each user should just know what fields to fill in in what
combinations becomes too fragile.
If I keep going down this route, a next phase might be to provide
accessor functions for reading the structure. Not yet sure how necessary
that is, but the sort of thing I'm thinking is that the old/theirs/mine
files might not necessarily be stored and retrieved as temporary file
names, but rather stored as references to the WC entry and later
retrieved as svn_stream_t's, or something like that. In the limit, it
might be good for the structure itself to become opaque, non-public.
Basically, this feels like a good way to go, but we haven't done much of
this from what I can see so I wanted a sanity check. I'm also not sure
how far it is worth taking the idea. Probably not too far.
I may pursue this a bit further and then show a complete patch.
Thanks.
- 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-08-29 01:02:36 CEST