Skip to content

Ruby: encode repeated field in packed format if desired#7381

Closed
qnighy wants to merge 1 commit intoprotocolbuffers:masterfrom
qnighy:ruby-packed
Closed

Ruby: encode repeated field in packed format if desired#7381
qnighy wants to merge 1 commit intoprotocolbuffers:masterfrom
qnighy:ruby-packed

Conversation

@qnighy
Copy link
Contributor

@qnighy qnighy commented Apr 15, 2020

This patch adds packed: true / packed: false options to repeated descriptor declaration, and uses that information when encoding packable fields.

    add_message "basic_test.TestMessage" do
      optional :optional_int32, :int32, 1
      optional :optional_int64, :int64, 2
      optional :optional_uint32, :uint32, 3
      optional :optional_uint64, :uint64, 4
      optional :optional_bool, :bool, 5
      optional :optional_float, :float, 6
      optional :optional_double, :double, 7
      optional :optional_string, :string, 8
      optional :optional_bytes, :bytes, 9
      optional :optional_msg, :message, 10, "basic_test.TestMessage2"
      optional :optional_enum, :enum, 11, "basic_test.TestEnum"
      repeated :repeated_int32, :int32, 12, packed: true
      repeated :repeated_int64, :int64, 13, packed: true
      repeated :repeated_uint32, :uint32, 14, packed: true
      repeated :repeated_uint64, :uint64, 15, packed: true
      repeated :repeated_bool, :bool, 16, packed: true
      repeated :repeated_float, :float, 17, packed: true
      repeated :repeated_double, :double, 18, packed: true
      repeated :repeated_string, :string, 19
      repeated :repeated_bytes, :bytes, 20
      repeated :repeated_msg, :message, 21, "basic_test.TestMessage2"
      repeated :repeated_enum, :enum, 22, "basic_test.TestEnum", packed: true
    end

TODO: implementation for JRuby

@haberman
Copy link
Member

I'm somewhat reluctant to keep going down the path of adding more and more complexity to the DSL. Even if we fix this, we already know there are N more things that the DSL still doesn't cover.

Worse, the DSL will never be able to capture 100% of the information in a protobuf descriptor. Custom options can be self-referential, so it in some cases it will not be possible to specify these cleanly in the DSL.

If the generated code simply embedded a binary descriptor, we would have perfect fidelity and we would never have to fix the DSL again. But we'd give up the readability of the DSL. Perhaps the .proto file could be embedded in comments?

@acozzette acozzette closed this Oct 13, 2021
@qnighy qnighy deleted the ruby-packed branch October 13, 2021 02:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants