Re: GPatternSpec

[Owen Taylor <otaylor redhat com>]

Tim Janik <timj@gtk.org> writes:

> On 27 Apr 1999, Owen Taylor wrote:
> 
> > 
> > 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.
> 
> i think your are black and white painting things here too much.
> if we want people to still be using the 1.2.x series in a few months,
> we have to add/back-merge new features into glib along the way,
> or they'll switch to the current development version to gain
> functionality.
> i hashed that out in more detail on the "Re: glib-1.3" thread
> earlier this month, and i'm sure you still remeber that.

Of course I remember it.

I think GLib as presented in 1.2.x is a reasonably complete
and useful facilities. Yes, backporting changes into GTK+
may be a useful technique for keeping people from switching
to the development version .... but it needs to be done
in a controlled fashion.

- First, the features that were added after 1.2.0 have to
  be PROMINENTLY labelled as such in the documentation.
  The developer needs to know that by using these features,
  they are requiring their user's to install an extra
  package - and one, that, if not installed correctly, could
  potentially break the user's distribution.

- Second, there needs to be quality control as good
  as the QC we did for 1.2.

- Third, we have to be sure that the API is pretty much
  exactly how they want it. Any API's that appear in 1.2 must 
  be left in until 1.4, and probably should be left
  in for 1.4 as well.

I guess what convinces me most that introducing changes 
in 1.2 is a bad idea is looking at some of the language
bindings. (This applies more to GTK+ than GLib.) Most
of them are still now in the process of catching up
to the 1.2 API's. We have an obligation to provide
people with a stable, unchanging target.

 
> > (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...)
> 
> i didn't mean to document this as not usable, especially since
> we are talking about fairly well tested code here already. the
> only thing required to make proper use of such new glib features
> is up'ing your AM_PATH_GLIB() version number to 1.2.3.

The average developer, for a package aiming at a wide range
of users should be using exactly those features that appeared
in 1.2.0. 

> > > /* --- 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? ;-)
> 
> you could asks that for the gtk variants as well, and actually, i've
> already explained the interface somewhat detailed in some gtk-list
> messages. shouldn't be much of an issue to find that again, reformat
> it and put it in any place you'd tell me to find plausible.

I was suggesting two things:

 - Now that we have pretty complete API documentation for GLib;
   anybody who introduces new functions should be obliged to
   submit the appriate documentation for the RDP.

 - The set of functions wasn't very obvious to me at the header
   file leve.
   
> > 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.
> 
> compare this with the Gtk pattern specification variant:
> 
> struct _GtkPatternSpec
> {
>   /* keep this in sync with GPatternSpec */
>   GtkMatchType   match_type;
>   guint          pattern_length;
>   gchar         *pattern;
>   gchar         *pattern_reversed;
> 
>   gpointer       user_data;
>   guint          seq_id;
> };
> 
> at least for gtk's purposes, additional data needs to be added to the
> pattern spec, in general if you'd want to bundle such structures with
> some of yours, you'd do
 
What we use currently in GTK+ shouldn't be a constraint
on providing a good API in GLib. The GTK+ API pattern
stuff is pretty much an internal thing. 

> struct _Foo
> {
>   GPatternSpec pspec;
> 
>   /* else stuff */
>   gint     baz;
>   gboolean blob;
> };
> 
> for which the actuall structure size (and contents if you don't want to
> s/*/dummy/ for all of the structure fields) needs to be known.

struct _Foo
{
  GPatternSpec *psec;
  ...;
}
 
Yes, there is a bit of extra memory-management traffic here,
but it won't be the performance bottleneck. 

 > 
> > Also, it should be:
> > 
> >  g_pattern_spec_match().
> >  g_pattern_spec_match_string().
> >            ^^^^
> 
> that's an argument that you could have made when i added the pattern
> specs to gtk. 

I didn't care much about the pattern specs in GTK+ because
the assumption was that they were pretty special-purpose API's.
In GLib, the assumption is that they should be accessible
and understandable to anybody. 

> actually i was less thinking of the function names to
> denote the structure they are operating on (since those are no objects),
> but mere of a general "pattern matching mechanism" which would therefore
> introduce g_pattern_match, g_pattern_match_string and
> g_pattern_match_simple.
> in that context, GPatternSpec is more or less an auxillary structure that
> one would like to setup for pattern matching if performance matters.

Hmmm, basically, I think as soon as you have parts that are
not useable to the user (which GPatternSpec does) then you
have stepped over the line from structure to object.

Regards,
                                        Owen