Skip to content

fix: support for empty repeated options (#1715) - #1716

Open
steinybot wants to merge 1 commit into
protobufjs:masterfrom
steinybot:fix/empty-repeated-options
Open

fix: support for empty repeated options (#1715)#1716
steinybot wants to merge 1 commit into
protobufjs:masterfrom
steinybot:fix/empty-repeated-options

Conversation

@steinybot

@steinybot steinybot commented May 12, 2022

Copy link
Copy Markdown
Contributor

Fixes #1715


test.test(test.name + " - message options (Empty)", function (test) {
var TestMessageOptionsMsg = root.lookup("TestMessageOptionsEmpty");
test.equal("(mo_rep_msg).rep_value" in TestMessageOptionsMsg.options, false, "should treat empty repeated option as missing");

@steinybot steinybot May 12, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the right thing to do for this case?

I would have thought that options would get defaults just like any other value but is that at a higher level than the parser?

@steinybot
steinybot force-pushed the fix/empty-repeated-options branch from 6904a93 to 843fada Compare September 8, 2022 22:50
@steinybot

Copy link
Copy Markdown
Contributor Author

Anyone able to review this?

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.

Empty options array is not parsed correctly

1 participant