Tommaso Cucinotta wrote:
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).
Yes, those two points are rather important, but the main advantage is
that you get access to the internal private structures: ChangeList,
FontList and InsetList.
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)
Yes, passing a paragraph was exactly what I had in mind. That's why I
call that binary search ;-)
We may also need to have a specialization for math only searching, or by
inset type searching.
IIUC, Paragraph would register itself in the passed DocIterator, right?
We may also want to start from a given position inside the paragraph (0
by default).
where inside FindOptions I would push at least:
- case sensitivity
OK. We could also add a 'font sensitivity' option.
- begin and end DocIterator (i.e., search restricted to a selection)
No, I would only pass the 'start' and 'end' pos_type which would by
default be 0 and size().
- search direction
- how to deal with changes and deletes
Addition should be searched of course and deletions should be ignored.
In the future we could probably add a method to search deleted text, but
that is not a priority now IMO.
The return type as DocIterator pos and match_length aims to provide the
exact (possibly inset-nested) position where the found text is,
OK.
and what is
the match length starting from there.
I don't understand this last point.
Now multi-par search (if one wanted it) would have to be managed
externally
anyway, or inside a Text::find().
Yes. But using new Paragraph::find() of course.
Abdel.
