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

DecoderConfig: Introduce IgnoreUntaggedFields #277

Merged
merged 1 commit into from Apr 20, 2022

Conversation

radeksimko
Copy link
Contributor

This is to enable us to more easily (safely & predictably) serialize various structs representing indexes in go-memdb, so that these can be displayed in a useful format in a (memdb) UI.

There are of course builtin indexes which wouldn't necessarily need such special decoding mode, but that assumes we gate the UI on a particular version of go-memdb which has these tags added. More importantly there are custom indexes downstream which may not have any tags and this makes decoded map unpredictable and hard to work with.

@mitchellh
Copy link
Owner

Given this is opt-in, I'm happy to include it. Thanks!

@mitchellh mitchellh merged commit 7c5722f into mitchellh:master Apr 20, 2022
@radeksimko radeksimko deleted the ignore-untagged-fields branch April 21, 2022 06:27
@sam3d
Copy link

sam3d commented May 15, 2022

This PR checks for an empty tag. Would it be better to have it check that the specified TagName is not defined?

@radeksimko
Copy link
Contributor Author

@sam3d That seems like a different use case / feature. TagName is used for the name of the tag, and defaults to mapstructure. In the use case I described we'd still like to declare our own tag name, such as ui and gather any fields which have that tag. You may want to create a separate issue and explain your use case in more detail there.

@sam3d
Copy link

sam3d commented May 16, 2022

@radeksimko I think part of my confusion is the meaning of "untagged", as it could imply both of the following:

  • The field does not have the mapstructure tag given by TagName/default tag name, or
  • The field has no tags at all

As many of my structs also have json tags, the fields will never be considered untagged. Which seems weird to me, because why would this library care about what other tags I have on my structs that aren't related to mapstructure?

The use case would simply be that I want to have any struct fields that don't have a mapstructure tag to be ignored by default. Which I think is your use case too? It's just that these structs also have other tags

Does that make sense? It may be that this is a different feature, but it's surprising behaviour given the name at first glance

@radeksimko
Copy link
Contributor Author

Maybe "untagged" in the name does not communicate the intent in the best way, but I can't think of a better name frankly.

Which seems weird to me, because why would this library care about what other tags I have on my structs that aren't related to mapstructure?

It doesn't, it will only check for TagName (which defaults to mapstructure), so unless you explicitly set TagName: "json", it won't care about json tags.

tagValue := f.Tag.Get(d.config.TagName)
keyName := f.Name
if tagValue == "" && d.config.IgnoreUntaggedFields {
continue
}

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.

None yet

3 participants