Skip to content
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

More compliant options parsing #242

Merged
merged 11 commits into from
Apr 1, 2024

Conversation

Lupus
Copy link
Contributor

@Lupus Lupus commented Mar 7, 2024

This PR introduces Pb_raw_option, which is a raw set of options as they are parsed from the .proto file. Pb_option is modified to actually be a compiled set of options. See https://protobuf.com/docs/language-spec#options for more details on how options are represented and thus should be parsed. Citing the example from that link here (same example is also added to tests to illustrate the new option parsing behavior and its correctness):

The example below demonstrates use of destructured messages and destructured lists:

option (google.api.http).custom.kind = "FETCH";
option (google.api.http).custom.path = "/foo/bar/baz/{id}";
option (google.api.http).additional_bindings = {
    get: "/foo/bar/baz/{id}"
};
option (google.api.http).additional_bindings = {
    post: "/foo/bar/baz/"
    body: "*"
};

The following is equivalent to the above, and instead uses a single message literal instead of de-structuring:

option (google.api.http) = {
    custom: {
        kind: "FETCH"
        path: "/foo/bar/baz/{id}"
    }
    additional_bindings: [
      {
          get: "/foo/bar/baz/{id}"
      },
      {
          post: "/foo/bar/baz/"
          body: "*"
      }
    ]
};

@Lupus
Copy link
Contributor Author

Lupus commented Mar 22, 2024

@c-cube hey, would you be able to take a look at this?

@c-cube c-cube merged commit 04b733b into mransan:br-4.0 Apr 1, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants