protoc plugin, repeated packed field support and many more

[hjagodzinski]

This pull request:

• adds a plugin for protoc (protoc plugin? #21) to support fully Protocol Buffers syntax, including new kid in the block, syntax 3 (Add support for the new proto3 syntax #39), fixing long-standing issues (Error when trying to compile proto file #8, PHP Notice when trying to compile a proto file and failure when creating output PHP file #10, Syntax error required uint32 Time=1; #25, proto-file parse error: "Syntax error extensions 100 to max;" #32, Problem with recursive types #34, Syntax error ]; #43, Can't compile file with own enum type #45, Edit the php protobuf compiler for compiling and referencing dependencies in their own directories #58, Fix compiler output file path issue by adding _path_join() to do the path join #59, bug, filter syntax declare in proto #64, Lack support for c-style comment for the proto file, which official compiler C++/Java/Python do support #69)
• adds a generation of PSR-0 compliant PHP classes (Compiler should create multiple class files #24, Generate php file under PSR-0 Namespace Autoloading convention #70)
• adds a support for a repeated packed field (I get the following error when trying to compile a proto, error '[packed=true]' #31),
• fixes a segmentation fault (Fixed segmentation fault when using the extension in Ubuntu 14.04 #38),
• makes README more user-friendly (I hope),
• fixes a stub,
• removes the SplEnum usage because in fact it was only partial.

[serggp]

Are you sure it's a good idea to add a dependency? I understand that composer is a good tool to manage dependencies, but some teams don't use it, and thus you are forcing them to use this manager. Current version of this project doesn't require composer or any other external dependency (except protobuf). So changing this will bring some additional and unnecessary work for such teams.

[hjagodzinski]

I had doubts about adding a dependency but then I thought - I don't see a point in reinventing the wheel. The libraries are out there for a reason - you don't have to redo what has already been done. The alternative is to create another command line parser which I don't like. I believe the composer is a way to work with dependencies and I don't see it to be a problem that someone doesn't use it in his project. It's fairly simple to use and because the protoc-gen-php is external to your project it doesn't mean you need to use composer in it too now.

[todorovhristo]

Sorry for the stupid comment, but I cannot fully understand - is this PR introduces associative array(map) support or no? I can only see a warning that it won't be fully supported but I cannot see what the limitations will be.

Regards,
Hristo

[hjagodzinski]

@todorovhristo your question is good 😉 A map field is represented as an array of MapFieldEntry objects instead of simply array of keys and values. Take a look at the generated class. This kind of behavior is described in a greater detail in the official documentation in the Backwards compatibility. I see now this warning might be confusing and I wonder whether it's actually needed. Any suggestions on that subject are more than welcome.

[todorovhristo]

Ok I see now. It will be probably better to link the official documentation alongside with the github issue tracker. Or at least update the issue in github, as now looks a little bit as they are done in this revision.

Apart from that great work on this, big thumbs up 👍

[hjagodzinski]

I like your suggestion. I changed the issue's description. I hope it's more clear now what the warning actually means.

Thanks!

[serggp]

@hjagodzinski You have changed compilation command from 'php protoc-php.php foo.proto' to 'php protoc-gen-php.php foo.proto'. I think it's better to keep backward compatibility and rename protoc-get-php.php to old style protoc-php.php.
When you renamed it you forced users to update their compiling scripts. It takes time and prevents some users to update their php-protobuf to the latest version.

[hjagodzinski]

@serggp I wanted to rename the script to a name it should have from the beginning, accordant to Google's convention. Moreover the arguments the script accepts are different now (they mimic as close as possible the ones accepted by protoc). My intention was to clean up this project at the cost of lack of backward compatibility. I kept in my mind not to go too far with it though. I don't see how renaming the script might prevent someone from migrating to the newer release. If I were interested in new release it would be a big deal for me to do it. Can you give some insight so I can understand your point?

On the other hand I created this pull request instead of pushing directly to give a chance to others to check it out. I haven't received any complaints regarding the script name change.