You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Karl Fogel <kf...@newton.ch.collab.net> on 2002/04/23 21:30:00 UTC

Re: svn commit: rev 1752 - trunk/subversion trunk/subversion/include trunk/subversion/libsvn_diff

> +typedef struct svn_diff_node_t svn_diff_node_t;
> +typedef struct svn_diff_tree_t svn_diff_tree_t;
> +typedef struct svn_diff_position_t svn_diff_position_t;
> +typedef struct svn_diff_lcs_t svn_diff_lcs_t;

Anything not publically exported in svn_diff.h will need the
double-underscore namespace protection.  I won't mention it beyond
this, as it applies to a bunch of other stuff in libsvn_diff/diff.c
and you can find them all pretty easily.

> +typedef enum svn_diff_type_e
> +{
> +  svn_diff_type_common,
> +  svn_diff_type_diff_workingcopy,
> +  svn_diff_type_diff_repository,
> +  svn_diff_type_diff_common,
> +  svn_diff_type_conflict
> +} svn_diff_type_e;
> +
> +struct svn_diff_t {
> +  svn_diff_t *next;
> +  svn_diff_type_e type;
> +  apr_off_t baseline_start;
> +  apr_off_t baseline_length;
> +  apr_off_t workingcopy_start;
> +  apr_off_t workingcopy_length;
> +  apr_off_t repository_start;
> +  apr_off_t repository_length;
> +};

Hmmm.  These make me wonder about the general approach... Why would an
internal diff library know anything about working copies versus
repositories?  IMHO, its callers should just hand it data_source_1 and
data_source_2 (plus possibly data_source_3 for diff3), and it will do
diffy things.  It doesn't need to know that one of these sources is
from the wc, and another represents the repository.

In other words, I think it would be better to write this diff library
as though you plan to see it used by lots of other applications, not
just by this one version control system.  That will result in the
cleanest interfaces and internal data flow.

I haven't reviewed the rest of the code, not with any rigor anyway, so
silence signifies only ignorance.

-K

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

RE: svn commit: rev 1752 - trunk/subversion trunk/subversion/include trunk/subversion/libsvn_diff

Posted by Sander Striker <st...@apache.org>.
> From: Karl Fogel [mailto:kfogel@newton.ch.collab.net]
> Sent: 23 April 2002 23:30

>> +typedef struct svn_diff_node_t svn_diff_node_t;
>> +typedef struct svn_diff_tree_t svn_diff_tree_t;
>> +typedef struct svn_diff_position_t svn_diff_position_t;
>> +typedef struct svn_diff_lcs_t svn_diff_lcs_t;
> 
> Anything not publically exported in svn_diff.h will need the
> double-underscore namespace protection.  I won't mention it beyond
> this, as it applies to a bunch of other stuff in libsvn_diff/diff.c
> and you can find them all pretty easily.

Ah, right...
 
>> +typedef enum svn_diff_type_e
>> +{
>> +  svn_diff_type_common,
>> +  svn_diff_type_diff_workingcopy,
>> +  svn_diff_type_diff_repository,
>> +  svn_diff_type_diff_common,
>> +  svn_diff_type_conflict
>> +} svn_diff_type_e;
>> +
>> +struct svn_diff_t {
>> +  svn_diff_t *next;
>> +  svn_diff_type_e type;
>> +  apr_off_t baseline_start;
>> +  apr_off_t baseline_length;
>> +  apr_off_t workingcopy_start;
>> +  apr_off_t workingcopy_length;
>> +  apr_off_t repository_start;
>> +  apr_off_t repository_length;
>> +};
> 
> Hmmm.  These make me wonder about the general approach... Why would an
> internal diff library know anything about working copies versus
> repositories?  IMHO, its callers should just hand it data_source_1 and
> data_source_2 (plus possibly data_source_3 for diff3), and it will do
> diffy things.  It doesn't need to know that one of these sources is
> from the wc, and another represents the repository.

Hmpf.  Yah, I kinda needed it to get the thing straight in my head.
I can change it to something more generic now.
 
> In other words, I think it would be better to write this diff library
> as though you plan to see it used by lots of other applications, not
> just by this one version control system.  That will result in the
> cleanest interfaces and internal data flow.

Yup.

> I haven't reviewed the rest of the code, not with any rigor anyway, so
> silence signifies only ignorance.

Np.  Thanks for the initial feedback.
 
> -K

Sander


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org