-
Notifications
You must be signed in to change notification settings - Fork 18k
reflect: back out multiple keys in key:value pair in struct tag #40281
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
Comments
Just noting that this would mean that struct tag keys cannot contain a comma character. The current documentation at https://golang.org/pkg/reflect/#StructTag is "Each key is a non-empty string consisting of non-control characters other than space (U+0020 ' '), quote (U+0022 '"'), and colon (U+003A ':')." So this would be a change. Of course, it's fairly unlikely that anybody is using a tag key with a comma. |
It could be a breaking change of course. |
I have a large corpus of Go code that I can scan at some point to see if anyone uses , in tag names in practice. |
What if we use a space-separated list instead of a comma-separated one?
The spaces form is more analogous to what happens in Go argument lists: The space form is also quite a bit less of a wall of text and thus easier to skim. Compare:
And unlike comma, spaces cannot appear in valid tags today, |
Space-separated is much better and it indeed looks like Go argument list. Regarding proposal: I haven't use more than 1 file tag (json or yaml) before, but last week have played with different config files and leaving all this verbose repetitions ( |
I must admit that space separated could be a good idea which won’t break anything. Personally I feel it less comprehensible than comma separated and in go we have comma between arguments ( Even with space separated, the main issue is resolved so it’s ok to me ! ;) |
OK, so with the syntax I suggested last Thursday (space-separated instead of comma-separated keys), there is no concern about backwards compatibility. There hasn't been much discussion otherwise. I did make some progress analyzing tags in my Go corpus last week, and I found that 0.06% of tags have commas in keys (outside quoted values). All of those are mistakes. So we could plausibly use comma and not break things, but the space is more in keeping with the current syntax anyway. Based on the discussion above, this seems like a likely accept (with spaces, not commas). To be clear, the new syntax would be:
|
No change in consensus, so accepted. |
Hi! I would like to work on that feature. Is it possible? :) |
Sure, go ahead. |
Change https://golang.org/cl/248341 mentions this issue: |
Change https://golang.org/cl/281237 mentions this issue: |
Change https://golang.org/cl/281973 mentions this issue: |
Change https://golang.org/cl/281515 mentions this issue: |
This reverts CL 277092. Proposal golang/go#40281 was initially accepted, then declined. Stop accepting the new, now once again invalid, format. For golang/go#40281 For golang/go#43083 For golang/go#43226 Change-Id: Ida97263048f3a048f904a844f577d8353e3a1afa Reviewed-on: https://go-review.googlesource.com/c/tools/+/281973 Trust: Ian Lance Taylor <iant@golang.org> Reviewed-by: Heschi Kreinick <heschi@google.com>
Change https://golang.org/cl/282412 mentions this issue: |
Proposal #40281 was initially accepted, but has now been declined. This CL removes most of the work done to implement it. Specifically this reverts CLs 248341, 274448, 274474, and 278392. For #40281 For #43226 Change-Id: I5a9ebb4d9cb5fb0962434b64c59beb8343030be5 Reviewed-on: https://go-review.googlesource.com/c/go/+/281515 Trust: Ian Lance Taylor <iant@golang.org> Run-TryBot: Ian Lance Taylor <iant@golang.org> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
Everything is backed out now. |
In particular bring in CL 201973, which reverts support for multiple keys in a struct tag. For #40281 For #43083 For #43226 Change-Id: I66e76639cbbca55bdbff6956acdb0a97650fdd31 Reviewed-on: https://go-review.googlesource.com/c/go/+/282412 Trust: Ian Lance Taylor <iant@golang.org> Run-TryBot: Ian Lance Taylor <iant@golang.org> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
This reverts CL 277092. Proposal golang/go#40281 was initially accepted, then declined. Stop accepting the new, now once again invalid, format. For golang/go#40281 For golang/go#43083 For golang/go#43226 Change-Id: Ida97263048f3a048f904a844f577d8353e3a1afa Reviewed-on: https://go-review.googlesource.com/c/tools/+/281973 Trust: Ian Lance Taylor <iant@golang.org> Reviewed-by: Heschi Kreinick <heschi@google.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
If you can be 5X more productive with this feature then it means you spend at least 80% of your time writing struct tags. |
What about this? type MyStruct struct {
Field1 string `json/bson/xml/form:"field_1,omitempty" other:"value"`
} |
The issue discussed here is the same... It's not a problem from the selected char but the splitting itself. So, using |
go-sftag Yet another StructTag type ConnParams struct {
Host string `json:"host" yaml:"host" toml:"host"`
Port int `json:"port,omitempty" yaml:"port,omitempty" toml:"port"`
Username string `yaml/json:"username,omitempty" toml:"username"`
Password string `toml/yaml/json:"password"`
Database string `toml/yaml/json:"database"`
Options map[string]interface{} `yaml/json:"options,omitempty" toml:"options"`
} |
The idea is good but how to make it work with |
Hello 👋
I'm actually boring to set the same value for multiple key in my struct tag. Maybe I missed something but here is the boring thing:
What I'm proposing is:
Thanks 👍
The text was updated successfully, but these errors were encountered: