Re: [Pan-devel] some patches

[Charles Kerr]

On Tue, Jan 07, 2003 at 12:36:23AM +0100, Christophe Lambin wrote:
> 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

Sounds good.

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

PanCallbacks are used for two modules that don't know about each other.
I think it's safe to let prefs knows about text's color scheme,
so the code is okay as-is.

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

Sounds good.

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

I disagree.  Articles generated from Pan should always follow the standard,
but we shouldn't be afraid to take a few extra steps to handle _reading_
articles whose signature delimiters are broken.  Again, I think a reasonable
guess like "if there's no real signature delimiter, and we have this two-dash 
line,
and there are fewer than N (6?) lines left in the article..." would be okay.

cheers,
Charles