opinion poll on text widget iterators

[Havoc Pennington <hp redhat com>]

Hi,

I'm considering reworking tktext-port/frootkxtindex.h (in GNOME CVS).
Advice sought, especially from language binding maintainers.

Right now I have an opaque reference-counted iterator object,
FrooTkxtIndex. The annoying thing about this object is that 
it sucks as an iterator. That is, if you have a loop:

FrooTkxtIndex* iter;
iter = froo_tkxt_buffer_get_char_index(buffer, 0);
while (iter != NULL)
{
  /* do stuff with iter */

  FrooTkxtIndex* freeme = iter;
  iter = froo_tkxt_index_next_char(iter);
  froo_tkxt_index_unref(freeme);
}

So, yuck.

Proposed alternative is:

FrooTkxtIndex iter;
froo_tkxt_buffer_get_char_index(buffer, 0, &iter);
while (froo_tkxt_index_next_char(&iter))
{
  /* do stuff with iter */
}

I have some misgivings here, however. 

Misgiving #1: the static, non-refcounted object will be like 
 GdkColor for language bindings, and may be annoying to deal 
 with. I don't know.

Misgiving #2: these index objects are 90% of the time not actually
 used for iteration; it will be both easier and more efficient to use
 _foreach() API's that are also provided for iterating over the
 buffer. "Dereferencing" a FrooTkxtIndex is O(n) where n is the length
 of the line containing the index. A typical use of FrooTkxtIndex is
 more like this:

static gboolean
not_whitespace(gint ch, gpointer user_data)
{
  return (ch != ' ' && ch != '\t');
}

static void
find_whitepace_region(const FrooTkxtIndex* center,
                      FrooTkxtIndex** start, FrooTkxtIndex** end)
{
  *start = froo_tkxt_index_find_char_backward(center, not_whitespace, NULL);
  *end = froo_tkxt_index_find_char_forward(center, not_whitespace, NULL);
}

or perhaps this:

static void
froo_tkxt_move_insert (FrooTkxt* tkxt,
                       FrooTkxtMovementStep step,
                       gint count)
{
  FrooTkxtIndex* insert;
  FrooTkxtIndex* newplace = NULL;

  insert = froo_tkxt_buffer_get_mark(tkxt->buffer, "insert");
  
  switch (step)
    {
    case FROO_TKXT_MOVEMENT_CHAR:
      newplace = froo_tkxt_index_forw_chars(insert, count);
      break;

    case FROO_TKXT_MOVEMENT_WORD:
      if (count < 0)
        newplace = froo_tkxt_index_back_word_starts(insert, -count);
      else if (count > 0)
        newplace = froo_tkxt_index_forw_word_ends(insert, count);
      break;

    case FROO_TKXT_MOVEMENT_LINE:
      break;

    case FROO_TKXT_MOVEMENT_PARAGRAPH:
      newplace = froo_tkxt_index_down_lines(insert, count);
      break;

    case FROO_TKXT_MOVEMENT_PARAGRAPH_ENDS:
      if (count > 0)
        newplace = froo_tkxt_index_next_newline(insert);
      else if (count < 0)
        newplace = froo_tkxt_index_start_of_line(insert);
      break;
    }

  froo_tkxt_index_unref(insert);
  
  if (newplace)
    {
      froo_tkxt_buffer_place_cursor(tkxt->buffer, newplace);
      froo_tkxt_scroll_to_mark(tkxt, "insert");
      froo_tkxt_index_unref(newplace);
    }
}

So misgiving #2 is that FrooTkxtIndex isn't actually inconvenient in
the common use case.

Misgiving #3: the static object is in some sense less opaque; the 
 size of the struct can't be changed while retaining binary
 compatibility, and the type can never be changed in a way 
 that requires a destructor (can never have allocated memory as 
 part of the iterator, basically).

Another alternative is simple character numbers, as in GtkText; right
now I support those, but they are less efficient for iteration than
the FrooTkxtIndex (they are O(log n) where n is the size of the
buffer, because you have to look up the char index in the btree, plus
the O(n) where n is line length once you find the proper line in the
tree). 

The character numbers are definitely going to be supported for common
operations, but I don't want them to be the only way to access the
buffer.

Anyway the relevant files are frootkxtbuffer.h, frootkxtindex.h, and
for example uses of the API see frootkxt.c.

Havoc