BUG in ParseQuery?

In Search/Build.html is sub ref ParseQuery with
these lines:

my $string  = shift;
my $tree    = shift;
my @actions = shift;
my $want    = KEYWORD | PAREN;
my $last    = undef;

Shouldn’t @actions be $actions, a ref to an array?
The sub goes on to push value onto the arrary but
the new values won’t be seen outside the sub.

If I am correct, it is easy to fix, but then shows
shows a deeper bug in the sub.

-Todd

In Search/Build.html is sub ref ParseQuery with
these lines:

my $string  = shift;
my $tree    = shift;
my @actions = shift;
my $want    = KEYWORD | PAREN;
my $last    = undef;

Shouldn’t @actions be $actions, a ref to an array?
The sub goes on to push value onto the arrary but
the new values won’t be seen outside the sub.

If I am correct, it is easy to fix, but then shows
shows a deeper bug in the sub.

That looks like a bug. assuming it’s being called with an arrayref

In Search/Build.html is sub ref ParseQuery with
these lines:

my $string  = shift;
my $tree    = shift;
my @actions = shift;
my $want    = KEYWORD | PAREN;
my $last    = undef;

Shouldn’t @actions be $actions, a ref to an array?
The sub goes on to push value onto the arrary but
the new values won’t be seen outside the sub.

If I am correct, it is easy to fix, but then shows
shows a deeper bug in the sub.

That looks like a bug. assuming it’s being called with an arrayref

Yes, it is. I tried changing it to: my $actions = shift

and changing all the “push @actions …” to “push @$actions …”

but then the sub breaks really bad. If you add a custom field
to the query and then do anything that causes parse to run
you get sent to the Advanced page to fix the not really
broken query. Ugly. :slight_smile:

Todd Chapman wrote:> On Wed, Nov 09, 2005 at 03:34:44PM -0500, Jesse Vincent wrote:

On Wed, Nov 09, 2005 at 04:01:28PM -0500, Todd Chapman wrote:

In Search/Build.html is sub ref ParseQuery with
these lines:

my $string = shift;
my $tree = shift;
my @actions = shift;
my $want = KEYWORD | PAREN;
my $last = undef;

Shouldn’t @actions be $actions, a ref to an array?
The sub goes on to push value onto the arrary but
the new values won’t be seen outside the sub.

If I am correct, it is easy to fix, but then shows
shows a deeper bug in the sub.

That looks like a bug. assuming it’s being called with an arrayref

Yes, it is. I tried changing it to: my $actions = shift

and changing all the “push @actions …” to “push @$actions …”

but then the sub breaks really bad. If you add a custom field
to the query and then do anything that causes parse to run
you get sent to the Advanced page to fix the not really
broken query. Ugly. :slight_smile:

Ok, I’ve now also had time to look at this. I don’t think the breakage
is that bad. All you really need to do is teach the parser about CFs.
Here’s a suggestion for a patch. I’m not quite sure if I hit the
preferred way to test for a custom field, but it seems to work for me.

Let me know what you think and whether I should file a bug report.

Rolf.

search.patch (2.13 KB)

Todd Chapman wrote:

In Search/Build.html is sub ref ParseQuery with
these lines:

my $string = shift;
my $tree = shift;
my @actions = shift;
my $want = KEYWORD | PAREN;
my $last = undef;

Shouldn’t @actions be $actions, a ref to an array?
The sub goes on to push value onto the arrary but
the new values won’t be seen outside the sub.

If I am correct, it is easy to fix, but then shows
shows a deeper bug in the sub.

That looks like a bug. assuming it’s being called with an arrayref

Yes, it is. I tried changing it to: my $actions = shift

and changing all the “push @actions …” to “push @$actions …”

but then the sub breaks really bad. If you add a custom field
to the query and then do anything that causes parse to run
you get sent to the Advanced page to fix the not really
broken query. Ugly. :slight_smile:

Ok, I’ve now also had time to look at this. I don’t think the breakage
is that bad. All you really need to do is teach the parser about CFs.
Here’s a suggestion for a patch. I’m not quite sure if I hit the
preferred way to test for a custom field, but it seems to work for me.

Let me know what you think and whether I should file a bug report.

Rolf.

It’s already filed as bug # 7133.

Since I am refactoring this stuff into a module, I’ll include your
changes there. That way it is reasonable easy to regression test it.

-Todd