On Tue, Jan 20, 2009 at 3:58 PM, Helder Suzuki <heldersuz...@gmail.com> wrote: > > I sent a partial patch a while ago and I disappeared without completing the > text_format in python, sorry about that. > So far I've only implemented the tokenizer part (w/ test cases), but anyone > is free to use it to implement the parser part (I'd be really glad), and for > some reason I couldn't set up the codereview (I didn't try hard though). For > various reasons I won't be able to touch it in the next few weeks. > This is such an important feature, specially if you use protobufs for > configuration files, it'd be so handy!
Can you sign the contributor agreement: http://code.google.com/legal/individual-cla-v1.0.html It's required for us to accept your patch (each contributor has to sign it). I have reviewed you patch but by some reason I'm unable to send you the comments. We have 2 choices: 1. I can apply my own code review comments and submit it in the internal repository then I will write the rest of the ParseASCII code. 2. You can apply the code review comments below and I'll submit the patch in external SVN. In any case you have to sign the agreement above. Here are my comments on the patch: google/protobuf/text_format.py: -__all__ = [ 'MessageToString', 'PrintMessage', 'PrintField', 'PrintFieldValue' ] +#__all__ = ['MessageToString', 'PrintMessage', 'PrintField', 'PrintFieldValue'] (Draft) 2008/12/08 18:37:10 Why is this line commented? +class Token(object): + """TODO(helder): Document me.""" (Draft) 2008/12/08 18:37:50 Please add the docstrings. +class Tokenizer(object): + """TODO(helder): Document me.""" (Draft) 2008/12/08 18:39:09 1-2 sentences will be enough. + def ConsumeNumber(self, started_with_zero, started_with_dot): + is_float = False + if started_with_zero and self.TryConsumeOneOf(('x', 'X')): + # A hex number (started with "0x"). + self.ConsumeOneOrMore(HexDigit, '"0x" must be followed by hex digits.') + elif started_with_zero and self.LookingAt(OctaDigit): (Draft) 2008/12/08 19:05:42 LookingAt(OctaDigit) -> LookingAt(Digit). It isn't a bug but a hidden feature. This will accept 09 as a decimal 9 (will proceed with the next case - "decimal number"). I think in this case we want to error out with a message like the one below - "Numbers starting with leading zero must be in octal.". That's what Kenton's implementation does. google/protobuf/internal/text_format_test.py: + def testParseInteger(self): + self.assertEqual(text_format.ParseInteger('0'), 0) + self.assertEqual(text_format.ParseInteger('1'), 1) + self.assertEqual(text_format.ParseInteger('012345'), 5349) # base 8 (Draft) 2008/12/08 19:41:49 Can you also add a test which tries to parse 09. It should result in an error. > > > Thanks, > Helder > > On Tue, Dec 30, 2008 at 12:19 AM, Piotr Findeisen <piotr.findei...@gmail.com> > wrote: >> >> >> >> On Dec 8, 7:27 pm, Petar Petrov <pe...@google.com> wrote: >> > On Mon, Dec 8, 2008 at 10:21 AM, Kenton Varda <ken...@google.com> wrote: >> > > Hey Petar, isn't there a patch someone was trying to submit that >> > > implements >> > > text format parsing? (For real, not by wrapping protoc.) What's the >> > > status >> > > of that? >> > >> > I'll review it today. >> > Hopefully the author hasn't forgotten about it. >> >> Hey! >> This gonna be a feature I miss really much! >> Is there happening anything about this? >> >> regards! >> Piotr >> >> >> > --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Protocol Buffers" group. To post to this group, send email to protobuf@googlegroups.com To unsubscribe from this group, send email to protobuf+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/protobuf?hl=en -~----------~----~----~----~------~----~------~--~---
