-
Notifications
You must be signed in to change notification settings - Fork 15.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[docs] Issues with proto2 and proto3 language specifications #1148
Comments
I think the service definition can be written even clearer:
|
More information on the "Aggregate Syntax" here: protocolbuffers/protobuf#1148 These are accepted by the proto compiler but rarely used - I found an example in the [Cloud Endpoints docs](https://cloud.google.com/endpoints/docs/grpc-service-config/reference/rpc/google.api#google.api.HttpRule.FIELDS.string.google.api.HttpRule.rest_method_name). Fixes protobufjs#383.
More information on the "Aggregate Syntax" here: protocolbuffers/protobuf#1148 These are accepted by the proto compiler but rarely used - I found an example in the [Cloud Endpoints docs](https://cloud.google.com/endpoints/docs/grpc-service-config/reference/rpc/google.api#google.api.HttpRule.FIELDS.string.google.api.HttpRule.rest_method_name). Fixes protobufjs#383.
|
We triage inactive PRs and issues in order to make it easier to find active work. If this issue should remain active or becomes active again, please add a comment. This issue is labeled |
We triage inactive PRs and issues in order to make it easier to find active work. If this issue should remain active or becomes active again, please reopen it. This issue was closed and archived because there has been no new activity in the 14 days since the |
I would like to humbly request that the Protocol Buffers Reference pages be updated to reflect the current language definition(s): specifically, to correct the following issues with the Proto2 and Proto3 language specifications.
Issues identified include the following.
Proto2 Service definition section indicates that "stream ( , )" is a valid construct. However, this construct is supported by neither protoc nor the FileDescriptor definition (nor has it ever been going back to the first GITHUB commit):
Original:
Recommended Edit:
Proto3 Service definition section references the "stream" production described above in the definition of "service", but deletes the definition of "stream". Looks like the section was copied from Proto2 and while "stream" definition was deleted, the reference on the "service" line was not updated.
Original:
Recommended Edit:
"constant" is not defined for either proto2 or proto3.
The value 'constant' is used in several places, but is not defined. This appears to be a literal value (int, float, bool, or string), but that is an assumption because it is never explicitly defined
For example:
Recommended Edit on both proto2 and proto3 language specification pages:
Between "String literals" and "EmptyStatement", add the following:
Option "Aggregate Syntax" is neither documented nor defined.
Search for Aggregate Syntax on the Proto2 Language Guide page. The example usage defines an aggregate syntax for option definition that is supported by the protoc compiler, but which is neither well documented in the language guide nor defined on the proto2/proto3 language specification pages.
Original for both proto2 and proto3:
Recommended Edit for both:
Also, update the example referenced above to include a comma separator between values.
NOTE: have not verified if the altOptSyntax is actually supported by the current protoc; to my recollection it is.
Parser appears to support negative integer and and floats literals in an option definition, but this is not reflected in the language specification.
According to the language specification, all intLit and floatLit values must be positive values (minus sign is not supported):
Integer definitions in general should get a review pass. E.g.,
intLit
is is defined for all integer literals, including the definition offieldNumber
. The former allows 0, but the latter appears to require a positive value >= 1.Example at the end of proto2 language spec is incorrect:
Should read:
The text was updated successfully, but these errors were encountered: