Hello Jean-Marc, > > I continue my backwards visit of history.
Although I posted some replies to mailing list last week, they do not seems to appear to the outside yet. So I am re-sending those replies. I carefully followed your reviews and modified the code accordingly. Your comments on them were really useful. > I forgot a couple of remarks in this commit. > > - RowPainter rp(pi, *text_, pit, row, bidi, x, y); > + > + cur.setTargetX(); > > Do you really need to set targetX? This seems plain wrong to me since, > if it was set to some useful value, you are just killing this value. As I have removed the too_wide_offset_ value modification from that method, I think it is better to remove. Then hopefully the constant method "void Cursor::setTargetX() const" implemented by myself can be removed from Cursor.cpp and Cursor.h. > + BufferView & bv = cur.bv(); > + > > You are defining bv, whereas you already have bv_, which is a BufferView > const *. Advice: always check whether a variable is needed before > defining it. > > However in this case the situation is different since you want a > non-const cursor. A way to have is is to get a non-const BufferView. It > often happens in such methods that extra data is provided in ancillary > parameters, in this case pi (PainterInfo). > > In particular in pi you have pi.bv, which is a pointer to the buffer > view. Therefore you can change the definition of cur to > Cursor & cur = pi.bv.cursor(); But I think changing the definition of cur is not suitable, because that exists there and as its definition is like this: Cursor const & cur; in this drawParagraph() method cur is used with most of the constant methods. So, or else should we define another Cursor variable which is non-constant? Thank you Hashini
