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

export map key and value properties in proto.Properties #312

Closed
jhump opened this issue Mar 16, 2017 · 3 comments
Closed

export map key and value properties in proto.Properties #312

jhump opened this issue Mar 16, 2017 · 3 comments

Comments

@jhump
Copy link
Contributor

jhump commented Mar 16, 2017

Right now, any code that wants to use reflection to crawl a generated struct has to re-parse the "protobuf_key" and "protobuf_val" struct tags on each encounter of a map field. They are not available via the properties one gets back from a call to proto.GetProperties(reflect.Type).

But the proto.Properties struct (returned from proto.GetProperties) has already parsed these tags. They are in unexported fields: mkeyprop and mvalprop.

With this issue, I propose either these two fields be exported. As an alternative, make the same information available through another facility in the proto.StructProperties API.

@bcmills
Copy link
Contributor

bcmills commented Mar 16, 2017

I don't know the reasoning behind which Properties fields are exported vs. not. (If we were to export mkeyprop and mvalprop, we should probably do the same for sprop and perhaps proto3 and oneof.)

@dsymonds, I know you're not really working with the proto library any more, but do you have any insight into the reasoning behind this part of the API?

@dsymonds
Copy link
Contributor

Most of Properties is unexported because it is considered an implementation detail of the library. In that regard, unexported is the default state. The parts that are exposed are usually because they're useful in other places, though some of the time it's accidental or a historical mistake.

It's probably okay to export a few more of the fields, though the comment on the entire struct could probably be expanded to remind people not to change things

@dsnet
Copy link
Member

dsnet commented Feb 14, 2018

Closing this in favor of a proper proto reflection API (#364).

@dsnet dsnet closed this as completed Feb 14, 2018
@golang golang locked and limited conversation to collaborators Jun 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants