Re: GPatternSpec

[Owen Taylor <otaylor redhat com>]

Tim Janik <timj@gtk.org> writes:

> hi all,
> 
> i've renamed the GtkPatternSpec API/implementation for GLib now,
> and wondered whetehr there are any objections to incorporare this
> into the 1.2 tree. the code is obviously fairly well tested already,
> it works nice for gtk's key bindings and rc-file lookups.
> to stay backwards compatible, we only need to alias the
> GtkMatchType enum values ala GTK_MATCH_ALL = G_MATCH_ALL and
> keep GtkPatternSpec in sync with GPatternSpec (GtkPatternSpec
> contains the additional fields user_data and seq_id).
> since the code will go into 1.3 anyways, and i don't expect it
> to change even after that, i don't see a good reason for not
> putting it into 1.2.3.

There is a very good reason not to put _any_ new features
into 1.2.x. No-matter how compatible. We want developers
to write for 1.2.0. (plus bug fixes) We don't want developers
to write for 1.2.x (x >= 23). 

I would consider it a good thing if a lot of people are
still using 1.2.2 in 5 months. Developers should have
confidence that they can use all the features of GTK+-1.2
and have to worry that their users will have the wrong
1.2.

(Obviously, if we add features, then document them as
"don't use this", it has almost the same effect - but
then there was no reason to add them and risk introducing
bugs in the first place...)

> /* --- GLib pattern matching --- */
> 
> /* GPatternSpec match types */
> typedef enum
> {
>   G_MATCH_ALL,       /* "*A?A*" */
>   G_MATCH_ALL_TAIL,  /* "*A?AA" */
>   G_MATCH_HEAD,      /* "AAAA*" */
>   G_MATCH_TAIL,      /* "*AAAA" */
>   G_MATCH_EXACT,     /* "AAAAA" */
>   G_MATCH_LAST
> } GMatchType;
> 
> /* Pattern matching
>  */
 > typedef struct  _GPatternSpec GPatternSpec;
> 
> struct _GPatternSpec
> {
>   GMatchType match_type;
>   guint      pattern_length;
>   gchar     *pattern;
>   gchar     *pattern_reversed;
> };
> 
> void     g_pattern_spec_init      (GPatternSpec       *pspec,
>                                    const gchar        *pattern);
> void     g_pattern_spec_free_segs (GPatternSpec       *pspec);
> gboolean g_pattern_match          (GPatternSpec       *pspec,
>                                    guint               string_length,
>                                    const gchar        *string,
>                                    const gchar        *string_reversed);
> gboolean g_pattern_match_string   (GPatternSpec       *pspec,
>                                    const gchar        *string);
> gboolean g_pattern_match_simple   (const gchar        *pattern,
>                                    const gchar        *string);

Where's the documentation? ;-)

Actually, I'd much rather see an opaque DT.

typedef struct  _GPatternSpec GPatternSpec;

GPatternSpec *g_pattern_spec_new     (const char *pattern);
void          g_pattern_spec_destroy (GPatternSpec *pattern);
...

I don't see any reason to expose the structure internals 
or GMatchType.

Also, it should be:

 g_pattern_spec_match().
 g_pattern_spec_match_string().
           ^^^^

Regards,
                                        Owen