Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

expose accessor for value of unregistered extensions [dev] #483

Merged
merged 3 commits into from
Jan 19, 2018

Conversation

jhump
Copy link
Contributor

@jhump jhump commented Jan 11, 2018

This is the same as #420, but for the dev branch. This would resolve #385.

@dsnet
Copy link
Member

dsnet commented Jan 12, 2018

Rather than adding a new function, can we change GetExtension to return the raw bytes as a []byte if the ExtensionDesc passed in is "partial"; where we define a partial ExtensionDesc as one where ExtensionDesc.ExtensionType is nil.

@dsnet
Copy link
Member

dsnet commented Jan 12, 2018

It seems to me that this would be a forward compatible change since the current behavior is to panic since GetExtension calls decodeExtension, which calls reflect.New(reflect.TypeOf(ExtensionDesc.ExtensionType)).Elem(), which results in a panic.

@jhump
Copy link
Contributor Author

jhump commented Jan 14, 2018

@dsnet, done. PTAL

@@ -293,14 +293,22 @@ func ClearExtension(pb Message, extension *ExtensionDesc) {

// GetExtension parses and returns the given extension of pb.
Copy link
Member

Choose a reason for hiding this comment

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

// GetExtension retrieves a proto2 extended field from pb.
//
// If the descriptor is type complete (i.e., ExtensionDesc.ExtensionType is non-nil),
// then GetExtension parses the encoded field and returns a Go value of the specified type.
// If the field is not present, then the default value is returned (if one is specified),
// otherwise ErrMissingExtension is reported.
//
// If the descriptor is not type complete (i.e., ExtensionDesc.ExtensionType is nil),
// then GetExtension returns the raw encoded bytes of the field extension.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@jhump
Copy link
Contributor Author

jhump commented Jan 19, 2018

@dsnet: I addressed your last comment. Is this ready to merge now?

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants