On Wed, 2009-06-24 at 10:14 +0200, Paweł Blokus wrote: > I have changed the code according to your suggestions(I will remove > the test code in the last commit). At some places I had doubts which I > explain below.
Thanks. Once you've done that, please say so, and I'll re-review the code. > > > + /* if the offset is a space at the end of the line set the caret > > > before > > > + it otherwise the caret will go on the beginning of the next line > > > + */ > > > + if (b_off == (unsigned)ta->lines[line].b_length && > > > + ta->text[ta->lines[line].b_start + > > > + ta->lines[line].b_length - 1] == ' ') > > > + c_off--; > > > > This seems odd. Can you explain why it's needed? > > The caret position is the character offset before which the caret is > positioned. Only a space or newline can be at the end of a line. A > newline doesn't belong to any line and the caret doesn't go to the > next line this way. A space however is counted as belonging to the > line which is higher so a caret after it would float to the beginning > of the next line. Ok, so this is to cater for lines which are soft-wrapped, yes? Perhaps something like this would help: /* If the calculated byte offset corresponds with the number of bytes * in the line, and the line has been soft-wrapped, then ensure the * caret offset is before the trailing space character, rather than * after it. Otherwise, the caret will be placed at the start of the * following line, which is undesirable. */ That said, however, perhaps we should give some thought to how to resolve the ambiguity of character indices when soft-wrapping occurs. > What should lines, that are already indented so much that it's hard to > wrap them, look like? The style guide didn't give me an answer to that > :) It's a sure sign the code is too complex and needs refactoring. > > > Index: gtk/gtk_plotters.c > > > =================================================================== > > > --- gtk/gtk_plotters.c (revision 7935) > > > +++ gtk/gtk_plotters.c (working copy) > > > @@ -119,7 +119,7 @@ > > > line_width = 1; > > > > > > cairo_set_line_width(current_cr, line_width); > > > - cairo_rectangle(current_cr, x0, y0, width, height); > > > + cairo_rectangle(current_cr, x0 + 0.5, y0 + 0.5, width, height); > > > > This seems orthogonal to the textarea changes. > > I've changed this in treeview too as it was drawing blurry rectangles. Yeah. It's unrelated to either the treeview or textarea changes, however (which was mostly my point). > Also, most of the unimplemented key presses deal with clipboard > handling. I was wondering if I should integrate it with what is > currently available in selection and create an unified API which would > have be declared in desktop/gui.h, create a local clipboard or not > implement it at all. Good question. I don't have an answer right now. Perhaps someone else has some thoughts. J.
