Re: Patch for gnatsweb.pl

[Lars Henriksen]

Hello,

Since no one else will, I'll comment myself ;-)

On Sun, Sep 22, 2002 at 08:47:33PM +0200, Lars Henriksen wrote:
> On Mon, Sep 16, 2002 at 09:41:21AM -0400, Robert Lupton the Good wrote:
> 
> > (This is my `patch.3b')
> [snip]
> >     1/ requires exact matches for field names selected using
> > scrolling lists.  For example, we have a "Scheduled" field that
> > has entries "approved" and "notapproved"; without this patch
> > selecting "approved" gets "notapproved" too.  The reason is that
> > gnatsweb uses a ~ match in its query, and this seems reasonable.
> > 
> > This patch adds explicit "^" and "$" to the fields, and then
> > adds code so that the user doesn't see them (Note: this doesn't
> > work very elegantly with my "load stored query" patch that
> > I sent to the list a few days ago.  This problem is fixed in
> > a later patch that I'll send today).
> 
> This is a clever trick, but I believe there is more to it.

I've realised that there is even more to it than I thought at first.

Robert Lupton's patch as well as mine breaks backward compatibility:
bookmarked and stored queries may stop working or behave differently.

According to comments in the gnatsweb code, that is also what has kept
the parameter/field/lower/upper case confusion from being dealt with and
field2param() and param2field() from disappearing. I have spent some
time on figuring out the use of (dbconfig) field names versus form
parameter (lower case) versions. It is not too difficult to eliminate
the parameter/field conversion entirely and use dbconfig names (I have
a patch) but again, not without breaking some existing bookmarked and
stored queries.

Do we want to (must we) keep backward combatibility? With gnatsweb 3.*?
With earlier gnatsweb 4 versions? As far as I can see from some bookmarks
produced by 3.113, compatibility with that version is already broken
(I have seen some "arrivedbefore=" or "modifiedafter=" query parameters
that are not recognised by gnatsweb 4).

Are we stuck with the broken logic in the following piece of code which
means that list selection on the advanced query page is interpreted
as a regular expression match whereas list selection on the query page
is interpreted as a string match (e.g. selecting state = critical will also
return non-critical from the advanced query page, but not from the query
page)?

> This affects the following piece of code in submitquery():
> 
>             # Most (?) people expect queries on enums to be of the
>             # exact, not the substring type.
>             if (fieldinfo($field, 'fieldtype') =~ "enum|multienum")
>             {
>               $subexp = appendexpr ($subexp, '|', "$ucfield==\"$sval\"");
>             }
>             else
>             {
>               $subexp = appendexpr ($subexp, '|', "$ucfield~\"$sval\"");
>             }
> 
> ...
> The upshot is that the if-clause is taken from query_page() (except for
> Synopsis), the else-clause from advanced_query_page() whether the field
> type is enumerated or not. If that was the intention, the comments and the
> if-expression are misleading to put it mildly!

What do you think?

Lars Henriksen