Re: [Pan-devel] some patches

[Christophe Lambin]

On Sun, 2003-01-05 at 11:43, Haran Shivanan wrote:
> I've attached patches to 3 source files. (text.c, text.h, prefs.c)
> (I'm assuming I send patches to the list. Please let me know if this is
> wrong)

Many thanks for these patches. Always happy to get them.

> Update the textview tags when you choose "apply" or "ok" in the
> preferences dialog and the colors have been changed.This allows you to
> instantly see the color changes.

Haven't looked in great detail, but I have the following comments:

1. set_text_buffer_tags() should be renamed to something like
'text_set_buffer_tags()', following the naming conventions in text.c

2. I suspect Charles would want to update the tags from prefs.c via a
PanCallback, but I have no big problem with a direct call to
text_set_buffer_tags().

3. At first glance, I suspect set_text_buffer_tags() leaks tags if the
tag already exists (i.e. allocated in set_text_buffer_tags, but only has
its values copied in gtk_text_tag_table_install; is never freed).

4. gtk_text_tag_copy_values() relies on the current definition of the
GtkText structure. I feel it would be better to have
gtk_text_tag_table_install() remove the old tag and install the new one
instead. This would also fix the memory leak in 3.

> Added syntax highlighting of signatures and a preferences option to
> choose the color for the signature.
> Not terribly important, I know...

It's been requested once or twice, so it would be nice to have.

> Made pan recognize "\n--\n" as a signature delimitor.
> I know this is not standards-compliant behaviour, but many news readers
> dont seem to follow the standards and it becomes annoying having to delete
> the sigs manually when creating a followup...

I'm not in favour of this, for the reasons Gollum mentioned: if we don't
follow the standard, we're bound to hit false positives. Following the
standards is the best approach.

Regards,
Christophe