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

Read json struct tags when toml tags are absent #880

Open
sparr opened this issue Jun 13, 2023 · 4 comments
Open

Read json struct tags when toml tags are absent #880

sparr opened this issue Jun 13, 2023 · 4 comments

Comments

@sparr
Copy link

sparr commented Jun 13, 2023

With go-toml v2 changing struct tag formatting to match the encoding/json package, it should now be possible to read and use json tags when toml tags are absent.

This behavior would reduce the amount of repetitive tagging required for cases such as opencontainers/image-spec#1073.

It might be optional, enabled by default, or always on. Each of these would have different impact on existing and future use cases.

@sparr
Copy link
Author

sparr commented Jun 15, 2023

Copied from my proposal linked above, where the "common tag" mentioned would just be the "json" tag if we use the idea in this issue.

Each of the packages could still read its own tag, for both unique and common functionality, with the following proposed conflict resolution behavior:

  • Specifying a field name in both the new common tag and the package tag would result in the package tag overriding the common tag.
  • Specifying an option (e.g. omitempty) in the common tag but not the package tag would result in the option still being applied; packages would need to provide an inverse option (e.g. keepempty) in their own tag to override this behavior.

Alternately, packages could read arbitrary options from the standard tag, which would simplify the struct definition even further but risks future collisions between options understood with different meanings by different packages.

@pelletier
Copy link
Owner

Thank you for the proposal! Sorry it took me a while to get back to you. I'm a little confused by opencontainers/image-spec#1073 (comment) this comment. Doesn't it mean that there is no need for even a toml tag since the lookup mechanism should already find those fields?

I'm concerned of defaulting to the json tag if it exists but the toml tag is not present. There could be valid reasons to want to slightly modify the behavior of the json encoder, but keep the default naming when encoding to toml. I think it would surprise me if a change to a json tag all the sudden would change the toml output. I understand the wish to reduce repetition, but explicitness makes things more predictable. I guess it could also be achieve by a linter to automatically update the yaml and toml tags based off of the json tag. Is there a similar proposal to streamline other encoding libraries?

@sparr
Copy link
Author

sparr commented Jul 14, 2023

First, the case insensitivity only works on the go side. If you use go json or toml to Marshal (as opposed to Unmarshal) you'll get matching case keys in the markup, which something on the other side probably won't honor.

Second, the automatic lookup only works for (case insensitive) exact matches. The example given there adds dots to the json and wants to add underscores to the toml, so the automatic keys won't match.

I think it would be cleaner for there to be a single standard struct tag that json/toml/yaml all honor and then have their own format-specific tags for unique behavior, which I've proposed in golang/go#60791 but that hasn't gained much traction. If that could happen, then I think it would be easier to get you and the (many) yaml library maintainers to honor it.

@pelletier
Copy link
Owner

Ah got it. I didn't see the proposal in the first thread.

If this proposal goes through, I'd be happy to have it implemented in go-toml! In the meantime, though, I don't think I am comfortable defaulting to an existing tag like json in the library. However, we now have a toml/unstable package. Maybe there is a way to expose a new encoder in that package that adds an extra option like SetTagName that allows the user to pick whichever tag name? That way, there is no public API change in go-toml, and that's available for those who need it.

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

No branches or pull requests

2 participants