Abdelrazak Younes ha scritto:
Hi Tommaso,
Hi,
Instead of adding this stringify() method, I would rather add a Paragraph::find() method. Then the actual searching would be done by the paragraph class that knows all that need to be known. This would also avoid the string copying all other the place.
I noticed this change has also been done for the basic find functionality.
From a design perspective, the old approach had the benefit that the
functionality was entirely external to Paragraph, instead now we are
going to move the functionality inside the Paragraph class (if smth. may
be done externally, why embeeding into the class ? -- it seems your argument
is performance and avoidance of wasteful copies).
This of course gives us more freedom on how it is internally implemented
(comprising the graph-based matching, if one wants).

Considering the API, I wouldn't recommend the current Paragraph::find()
signature, because it suffers from various drawbacks:

bool Paragraph::find(docstring const & str, bool cs, bool mw,
       pos_type pos, bool del) const

- what are you going to provide as str argument ?
 string-ified ? latex-ified ? lyx-ified ? xml-ified ?
- output is only a boolean, not a position of where it is found
- parameters are "spread"

Instead (and please comment on this) I would suggest something like:

void Paragraph::find(Paragraph const & par, FindOptions const & opt,
                           DocIterator & pos, int & match_length)

where inside FindOptions I would push at least:
- case sensitivity
- begin and end DocIterator (i.e., search restricted to a selection)
- search direction
- how to deal with changes and deletes

The return type as DocIterator pos and match_length aims to provide the
exact (possibly inset-nested) position where the found text is, and what is
the match length starting from there.

Now multi-par search (if one wanted it) would have to be managed externally
anyway, or inside a Text::find().

   T.

Reply via email to