Thanks Brock for clarification & providing the way to test the fix.
I tried the following approach to point out the position of the
offending character (I used the same lex rules that you posted in webrev).
Here is the sample o/p :
$ pkg search aaaaaab
.....
....
File "/usr/lib/python2.4/vendor-packages/pkg/query_parser.py", line 127,
in t_error
raise RuntimeError("%s has an unparseable character at position %d"
% (self.get_string(), self.get_pos() + 1))
RuntimeError: aaaaaab has an unparseable character at position 7
changes made in query_parser.py
gladiator@/mypool/Learnings/IPS/new-fix/fix-9693 $ hg pdiffs
diff -r 996f63549fe7 src/modules/query_parser.py
--- a/src/modules/query_parser.py Wed Jul 22 13:13:28 2009 -0700
+++ b/src/modules/query_parser.py Fri Jul 24 12:59:33 2009 +0530
@@ -122,7 +122,7 @@
return t
def t_error(self, t):
- raise RuntimeError("%s is a unparseable character")
+ raise RuntimeError("%s has an unparseable character at
position %d" % (self.get_string(), self.get_pos()))
def build(self, **kwargs):
self.lexer = lex.lex(object=self, **kwargs)
Please let me know if this fix looks good.
NOTE : This approach will only print the position of the first
occurrence of un-parsable char
Thanks,
~Saurabh
Brock Pytlik wrote:
Couple of things. The first is that we decided on a standard that said
only use parens when absolutely necessary, so the parens around
t.value should g. The other thing is that I'm not totally happy with
is what we're showing to the user. I think we can do better than just
displaying the character. t is actually a LexToken with the character
and some position information. I've posted a webrev at
http://cr.opensolaris.org/~bpytlik/ips-test/src/modules/query_parser.py.wdiff.html
which mangles the lexing rules so that searching for anything other
than a string consisting only of 'a' will error. (Ie, 'a', 'aa',
'aaaaaaaaaa' are all ok, but 'b', 'aaaaab', etc will fail.) That might
be helpful for your testing. What would be nice would be to actually
have the error point out the offending character, but that can be a
separate RFE if you don't want to repurpose this bug (if you do,
ParseError in that same file has an example of what I'm talking
about). In either case, I think we should convey the position
information to the user as well.
Brock
Saurabh Vyas wrote:
Brock Pytlik wrote:
Saurabh Vyas wrote:
Hi All,
Please review the fix for:
Bug-id : 9693 RuntimeError on line 125 of query_parser.py doesn't
fill in %s
webrev : http://cr.opensolaris.org/~saurabhv/fix-9693/
and let me know your comments.
Cheers,
~Saurabh
_______________________________________________
pkg-discuss mailing list
pkg-discuss@opensolaris.org
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Thanks Brock for reviewing and correcting this fix.
Yes it should be t.value which we should be printing (I tried
conforming this with the o/p, but could not tweak the rules to do so
:-( ).
Apart from the code point of view, I check the client/api.py where
the in-built error object is returned when the parsing exception occurs.
Earlier I assumed that the token itself is passed as 't' when the
parsing exception occurs (wrong to have such assumption ).
I have modified the webrev too, please have a quick review and let me
know if this looks good
http://cr.opensolaris.org/~saurabhv/fix-9693-01/
Thanks again,
~Saurabh
I'm not certain that t is what should be printed. I would guess that
it's t.value, but I'm not certain. Could you show us the output of
it? I'm also not sure that, given the current lexing rules, it's
actually possible to produce a lexing error at all, so it might be
necessary to tweak the rules a bit so that you can make this error
happen.
Thanks,
Brock
_______________________________________________
pkg-discuss mailing list
pkg-discuss@opensolaris.org
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
