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 .*\:.*.
- line 103: is "bar" a descriptive name, or just something other than
"foo"? Perhaps it could be called "fields", and "b" could be "field"?
Sure
- 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.
So, I think the answer is "no, it can't be simplified as you suggest"
but I'm not sure I understood the suggestion to be certain.
- 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).
- 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.
Thanks for the review,
Brock
?
Danek
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
