TRUE as an enum value in textprotos

[Max Kanat-Alexander]

So, per the textproto format spec, what I'm thinking here should work, but I want to see if there are gotchas in various textproto implementations that I might not be aware of. Yes, I could try it out myself, but there's no way I'm going to know all the sharp edges of every textproto implementation. :)

Let's say I have an enum like:

message MyStuff {

    enum MyBoolean {

        UNSPECIFIED = 0;

        TRUE = 1;

        FALSE = 2;

    }

}

message MyFile {

    MyStuff::MyBolean is_cool = 1;

}

And then I have a textproto that looks like this:

is_cool: FALSE

Per the textproto spec, the boolean "true" and "false" values are specific to boolean fields and are only parsed as booleans for boolean fields, so per the spec, that should work. However, will the textproto parser parse what I have written correctly as the enum FALSE, or might it accidentally interpret it as a boolean and set the value to 0?

(All that said, that enum might also run into problems in C++ because it conflicts with C++ macros, right? Sorry, I know I'm asking "Can I do something that the best practices say not to do," but it really would be helpful in my situation if I could do it. One thing I'm looking for here is an "out" for people who have specified a configuration as a boolean value and now want to move it to being an enum because it has more than two states, assuming that all data is textproto and never binary proto.)

-Max

[Em Rauch]

> All that said, that enum might also run into problems in C++ because it conflicts with C++ macros, right?

We actually defensively temporarily-undef a number of common macro names in C++Proto gencode so that the most common of these cases that you might want to use for an enum value don't break (like TRUE, DOMAIN, etc). I can see this test file that includes TRUE and FALSE, though the usage-site application code C++ might have to jump through hoops to either undef it themselves or spell it with one of the names that doesn't collide with the macro (thats out of our control):

 https://github.com/protocolbuffers/protobuf/blob/3acf23cce5aac088a7c6585484a8b38c48448255/src/google/protobuf/unittest_proto3_bad_macros.proto#L44-L45

By quick inspection on JavaProto and C++Proto's textproto parsers it should work (there's no phase to lex it first and then assign, at the moment it reaches that token it knows its trying to assign an enum not a boolean field and so looks at the name stringly).

So I think what you're suggesting seems sound to me; if you find the TextProto parse only fails in one language I think it would be an easy patch to fix and we would accept it.

--
You received this message because you are subscribed to the Google Groups "Protocol Buffers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to protobuf+u...@googlegroups.com.
To view this discussion visit https://groups.google.com/d/msgid/protobuf/CACHWaZ%3DE55HDfTvqHq9EbDftNBbO2SgdywRDwp5gHBA%3D6YVT2Q%40mail.gmail.com.

[Max Kanat-Alexander]

Fantastic, thanks so much! You just saved me like a whole day of trying to vet that myself. :)