Brock Pytlik wrote:
> On 04/ 1/10 03:07 PM, Danek Duvall wrote:
> >Brock Pytlik wrote:
> >
> >>http://cr.opensolaris.org/~bpytlik/ips-15284-v1/
> >query_parser.py:
> >
> > - line 100: this no longer returns a 3-tuple. It would be nice to
> > explain the regexp in English here, too.
> Thanks, I've updated the documentation. I can try to explain what the
> regexp is doing, but I'm not totally sure what you're looking for. I
> could go very simple like:
> The following regexp matches valid search terms with colons in them.
> Or, I could go into painful detail about why each of the restrictions
> is there (ie, why it's not .*\:.*.
I see effectively three groups -- the (optional) thing before the colon,
and two things after the colon which are separated by the "|" (FWIW, I
don't know whether the spaces surrounding the pipe are part of the actual
RE or whether that's some sort of PLY syntax).
I don't think you need to go into excruciating detail about each of the
restrictions, but talking about them at a high level might be useful. If
it's really painful to come up with something intelligible, then drop it.
> > - line 120: how do you end up with the empty string in bar[-1] other than
> > by having a terminal unescaped colon in the query? Could this test
> > then be simply on whether len(bar) == 1, and have the> 1 case set
> > token explicitly? Something like that would be easier to decipher, if
> > it can be correct.
> You end up w/ an empty string in bar[-1] exactly when you have a
> terminal escaped colon in the query.
>
> I'm confused by your question about the test. The following cases
> need to be handled
> query is ":a:b\:c" where bar=["", "a", "b:c"] so len of bar is >1 and
> token needs to be set but also when query is "b\:c" where bar =
> ["b:c"] so len of bar is =1 and token still needs to be set.
I guess what I'm thinking is (hand-waving):
token = bar[-1]
if len(bar) == 1:
t.type = self.reserved.get(token, "TERM")
t.value = token
else:
set key, action_type, and pkg_name
t.type = "FTERM"
t.value = ...
so token is always set (possibly to the empty string), and it's only t.type
and t.value which are set based on the number of fields in the query (which
is logically what's going on).
> > - line 133: you end up with "" instead of None for pkg_name, action_type,
> > and key if colons were in the query but no values were specified for
> > the fields, right? So why bother setting them to None earlier?
> Are you asking why not set them to "" or why set them period?
> Assuming it's the former, I don't think there's a good reason, at
> least not that I can think of at this point. I can change it, and
> change other places in the code not to check for None there as
> equivalent to "" (if I do that).
I think it's more that there's little point in setting them at all. Well,
key at least; pkg_name and action_type I guess need to be set.
> > - line 255: what do you mean by "attached"? Are we talking about
> >
> > foo:bar:baz:zap
> >
> > vs
> >
> > foo:bar:baz: zap
>
> No, we're talking about
> foo:bar:baz:z\:ap whee
> and
> foo:bar:baz:zap whee
> vs
> foo:bar:baz: whee
>
> The first two have a token attached to the FTERM (ie, its token value
> is not None). So the token field cannot be filled by whee. For the
> latter FTERM, it's token field is empty and what follows is a term
> that can fill that slot in the FTERM so it's folded in. That folding
> in is done as a crude (but lossless) optimization because it's
> substantially faster to complete a search when the token is not '*'
> for the leftmost FTERM.
I see. So
foo:bar:baz: whee
and
foo:bar:baz:whee
are identical searches. I wasn't aware of that.
Thanks,
Danek
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
