[Monotone-devel] Re: Sharing process_one_hunk (applying a rcs diff) between rcs_import and cvs_sync

[Christof Petig]

Graydon Hoare schrieb:
> On 10/7/05, Christof Petig <address@hidden> wrote:
> 
> 
>>- piece_store stores a vector of boost_shared_ptr<rcs_deltatext> and
>>processes them. So no way to work on plain strings (which are used
>>within cvs_sync). It is accessed via ->text.data(). If we could agree to
>>add a
>> const char *data() const { return text.data(); }
>>to rcs_deltatext, I could make piece_store a template to accept either
>>rcs_deltatext or pointer_emulating_string (see below) by using ->data()
> 
> 
> I don't understand what pointer_emulating_string does, or why you want

see below in my previous mail (it's an adaptor to string to match the
semantics of a shared_ptr) - unless we want to change the way
piece_storage works to work on vector<string>. (I would recommend that)

> access to a char*. When CVS ships you some RCS information, why can't

and const char * is simply the return value of std::string::data().

> you store it in an rcs_deltatext structure? Does it fail to provide
> you with num and log fields? Does it bother you to just leave those
> fields blank?

It simply bothers me that just to apply a diff I have to fake two
strings out of three. Perhaps this code gets more flexible if it does
not depend on creating a shared_ptr<rcs_deltatext>.

For example I use the index_deltatext function to separate the
certificate value into its lines.

>>Or we use a more invasive approach and pass only the text member to
>>piece_store (IIRC a string is only copied on write) and forget about
>>boost::shared_ptr. I don't see a benefit from using shared_ptr here (I
>>might overlook something).
> 
> 
> Well, a simple answer to that is that I'm using a shared_ptr because
> rcs_deltatext contains three strings, not one. Perhaps not a huge win,
> but it cuts the amount of refcount traffic by a third. Anyways, that's
> "why", in a sort of explaining-history sense. Let's discuss the
> future: whether you want to continue using that, or switch to
> something else, depends on why you feel we should not use a
> shared_ptr. Why not?

IIRC nothing in index_deltatext and construct_version prevents these
algorithm to work on plain strings. And constructing a string object
from a rcs_deltatext is as expansive as reffing a shared_ptr.

>>What should I name the new header file containing the template?
>>piece.hh? apply_rcs_diff.hh? line_spliced_string.hh?
> 
> 
> How about rcs_piece_table.{hh,cc} (you'll need a .cc file if you want
> to have a global variable constructed inside piece::). There is

(not if we speak about templates ;-) but it sounds like we don't need a
template)

> already an xdelta piece table implementation in xdelta.{cc,hh}, so
> it'd help if the filename actually contains the word "rcs" to
> differentiate it.

Agreed. Having _table in the name makes clear that it is a collection of
pieces.

>>Should I rename the class piece when I form a template? line_spliced_string?
> 
> 
> Perhaps rcs_piece. The literature on this algorithm calls it a piece
> table. I don't see a strong reason to change that word, only perhaps
> to clarify that it's an rcs piece, not an xdelta piece.

piece sounded just too generic to me, but rcs_piece_table is perfect.

Should I prepare an implementation so that we can discuss it? E.g. the
string vs rcs_deltatext issue should get clearer once the code is available.

   Christof

signature.asc
Description: OpenPGP digital signature