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.

2009/6/23 John-Mark Bell <[email protected]>
>
> On Tue, 2009-06-23 at 12:13 +0100, John-Mark Bell wrote:
> > +/**
> > + * get character offset from the beginning of the text for some coordinates
> > + *
> > + * \param ta Text area
> > + * \param x  X coordinate
> > + * \param y  Y coordinate
> > + * \return   character offset
> > + */
> > +static int textarea_get_xy_offset(struct text_area *ta, int x, int y)
> > +{
> > +     size_t b_off, c_off, temp;
> > +     int line_height;
> > +     int line;
> > +
> > +     if (!ta->line_count)
> > +             return 0;
> > +
> > +     line_height = css_len2px(&(ta->style->line_height.value.length),
> > +                                ta->style);
> > +
> > +     x = x - ta->x - MARGIN_LEFT + ta->scroll_x;
> > +     y = y - ta->y + ta->scroll_y;
> > +
> > +     if (x < 0)
> > +             x = 0;
> > +
> > +     line = y / line_height;
> > +
> > +     if (line < 0)
> > +             line = 0;
> > +     if (ta->line_count - 1 < line)
> > +             line = ta->line_count - 1;
> > +
> > +     nsfont.font_position_in_string(ta->style,
> > +                     ta->text + ta->lines[line].b_start,
> > +                     ta->lines[line].b_length, x, &b_off, &x);
> > +
> > +
> > +     for (temp = 0, c_off = 0; temp < b_off + ta->lines[line].b_start;
> > +                     temp = utf8_next(ta->text, ta->text_len, temp))
> > +             c_off++;
> > +
> > +     /* 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.

> > +     }
> > +}
> > +
> > +/**
> > + * Key press handling for text areas.
> > + *
> > + * \param ta The text area which got the keypress
> > + * \param key        The ucs4 character codepoint
> > + * \return           true if the keypress is dealt with, false otherwise.
> > + */
> > +bool textarea_keypress(struct text_area *ta, uint32_t key)
>
> Ensure code in this function is wrapped at 80 columns.
>

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
:)

> > +/**
> > + * Removes any CR characters and changes newlines into spaces if it is a 
> > single
> > + * line textarea.
> > + *
> > + * \param ta         Text area
> > + * \param b_start    Byte offset to start at
> > + * \param b_len              Byte length to check
> > + */
> > +void textarea_normalise_text(struct text_area *ta,
> > +             unsigned int b_start, unsigned int b_len)
> > +{
> > +     bool multi = (ta->flags & TEXTAREA_MULTILINE) ? true:false;
> > +     unsigned int index;
> > +
> > +     /*remove CR characters*/
> > +     for (index = 0; index < b_len; index++) {
> > +             if (ta->text[b_start + index] == '\r') {
> > +                     if (b_start + index + 1 <= ta->text_len &&
> > +                                     ta->text[b_start + index + 1] == 
> > '\n') {
> > +                             ta->text_len--;
> > +                             memmove(ta->text + b_start + index,
> > +                                             ta->text + b_start + index + 
> > 1,
> > +                                             ta->text_len - b_start - 
> > index);
>
> This is CRLF -> LF, right?
>
> > +                     }
> > +                     else
> > +                             ta->text[b_start + index] = '\n';
>
> CR -> LF.
>

Yes, it is. I've added the necessary comment.

> > 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.


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.

Reply via email to