Lars Gullik BjÃnnes wrote: > yes + some comment. > > | +using boost::prior; > > which header file include next_prior.hpp?
Couldn't find out. Should I add it just in case? > | +PosIterator & PosIterator::operator--() > | +{ > | + while (!stack_.empty()) { > | + { > > why the strange block? > > why not change the variable name or have a helper function? Ok. > | +PosIterator::PosIterator(ParagraphList * pl, ParagraphList::iterator > | pit, > | + lyx::pos_type pos) > | +{ > | + stack_.push(PosIteratorItem(pl,pit,pos)); > | +} > > space between args please. > > | + *this = PosIterator(par, pos); > > hmm... > > operator=(PosIterator(par, pos)); Done. > I don't know... I just find the *this = ... construct hard to read... > | + > | +#include "ParagraphList_fwd.h" > | + > | +#include <boost/scoped_ptr.hpp> > > Is the scoped_ptr used here? No, you're right. > I wonder if there is a way, so that we don't need to have those > XXX_iterator_(begin|end) functions. > > but ok for now. what about a pos_iterator_begin(Buffer &)? in PosIterator.[Ch] > | +PosIterator::PosIterator(ParIterator & parit, lyx::pos_type pos) > | +{ > > Why have this here? and not int he positerator.C file? Because I need ParPosition which is not defined in the header file. Should I put it on the header? > why do they have to be friends? Because I need private members of PosIterator in the PosIterator(Pariterator, pos) constructor. What should I be doing instead? Regards, Alfredo