-
Notifications
You must be signed in to change notification settings - Fork 18k
proposal: reflect: support tab and newline as tag separator #38641
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
Thanks. I turned this into a proposal. |
Although now that I've done that, I see that this is basically the same as #15893, which you mentioned. What is the difference between this proposal and that one? |
In general I like the implicit backpressure that the single-line requirement imposes: it keeps these things from turning into arbitrary pages-long annotation texts. I am not convinced we really want to open that door. The examples in the top comment are right at the limit of what seems reasonable. If we did allow multiline strings I guess gofmt would need to adjust the indentation of the continuation lines when it adjusts the padding of fields? |
Ultimately this comes down to a judgement call, not really a cost-benefit. But there is a real benefit to pushing back on such long struct tags - they persist at runtime in the binary, and they complicate trying to understand the program. Most uses of struct tags in the Go ecosystem are very short. Personally, I still believe that we should not encourage them to be very long, nor make that easier. We definitely cannot make gofmt align the multiline formatting (inserting or removing spaces inside the backquoted string), by the way, because in general gofmt can't be sure about the interpretation of these strings. /cc @myitcv @ianthehat @mvdan for more tools experience in this discussion. |
I agree with the last comment. Multiline string literals can also easily make struct types confusing or less readable, as there wouldn't be an indentation/format style that could be enforced. I think Go wouldn't have used a string literal for tags if they wanted tags to support complex data. It's really meant for a few simple attributes. If we really wanted we could add syntax for structural tags, akin to composite literals, but I imagine that would also encourage abusing tags for multiple purposes. |
Spitballing alternatives, would it be reasonable to have multiple field tags? Then the compiler count combine them with // Current record struct.
type Current struct {
ID int32 `db:"id,type=INT,auto_increment,key"` `binding:"gte=0"` `json:"id,omitempty"` `form:"id"` `uri:"id"`
Flag int32 `db:"flag,type=INT,not_null,primary"` `binding:"gte=0"` `json:"flag,omitempty"` `form:"flag"`
OS string `db:"OS,type=VARCHAR(16),not_null,primary"` `binding:"omitempty,oneof=AND IPH"` `json:"OS,omitempty"` `form:"OS"` `example:"AND"`
// ......
}
// Expect record struct, supporting one tag each line.
type Expect struct {
ID int32 `db:"id,type=INT,auto_increment,key"`
`binding:"gte=0"`
`json:"id,omitempty"`
`form:"id"`
`uri:"id"`
Flag int32 `db:"flag,type=INT,not_null,primary"`
`binding:"gte=0"`
`json:"flag,omitempty"`
`form:"flag"`
OS string `db:"OS,type=VARCHAR(16),not_null,primary"
`binding:"omitempty,oneof=AND IPH"`
`json:"OS,omitempty"`
`form:"OS"`
`example:"AND"`
// ......
} |
@carnott-snap even if adjacent back-quoted strings were somehow combined, splitting them across lines would not work because there would be an implicit semicolon after each string-at-end-of-line. (And you can't change the implicit semicolon after a string without breaking Thanks for adding your opinion @mvdan. Based on the discussion above, this seems like a likely decline. |
I am unclear how that invalidates the example, you could just update the parser to handle it. If your argument is that would be too expensive, then this becomes a readability vs parser elegance argument. If the lack of a block is troublesome, you could fix that too: // Expect record struct, supporting one tag each line.
type Expect struct {
ID int32 {
`db:"id,type=INT,auto_increment,key"`
`binding:"gte=0"`
`json:"id,omitempty"`
`form:"id"`
`uri:"id"`
}
Flag int32 {
`db:"flag,type=INT,not_null,primary"`
`binding:"gte=0"`
`json:"flag,omitempty"`
`form:"flag"`
}
OS string {
`db:"OS,type=VARCHAR(16),not_null,primary"
`binding:"omitempty,oneof=AND IPH"`
`json:"OS,omitempty"`
`form:"OS"`
`example:"AND"`
}
// ......
} |
@carnott-snap the rules for semicolon injection are part of the syntax, declared in the spec. Changing them isn't just about updating one parser or lexer. All Go developers have to adapt to the new rules, tools have to be fixed and updated, etc. It is indeed possible to propose changes to the Go language, but the bar for them is high, and you must fill a template to provide enough information for the reviewers: https://github.com/golang/proposal/blob/master/go2-language-changes.md |
I am not suggesting that the semicolons be removed, but that the parser understand that these are equivalent: one int32 `db:"id,type=INT,auto_increment,key"` `binding:"gte=0"` `json:"id,omitempty"` `form:"id"` `uri:"id"``
two int32 `db:"id,type=INT,auto_increment,key"`; `binding:"gte=0"`; `json:"id,omitempty"`; `form:"id"`; `uri:"id"` |
That is still a language change, since that syntax is not valid today. |
There's no change in the consensus since the last week. Declined. |
An ugly workaround: package main
import "fmt"
import "reflect"
type Expect struct {
ID int32 ` _:"
" db:"id,type=INT,auto_increment,key" _:"
" binding:"gte=0" _:"
" json:"id,omitempty" _:"
" form:"id" _:"
" uri:"id" _:"
"`
}
func main() {
t := reflect.TypeOf(Expect{})
id := t.Field(0).Tag
fmt.Println(id.Lookup("db")) // id,type=INT,auto_increment,key true
fmt.Println(id.Lookup("binding")) // gte=0 true
fmt.Println(id.Lookup("json")) // id,omitempty true
fmt.Println(id.Lookup("form")) // id true
fmt.Println(id.Lookup("uri")) // id true
} Though it might be a bug. |
Sometime structs are defined with mutlti tags, and long then 100 or 120 columns.
The code can be formated pretty if struct tag support spliting with blank or "\n" or "\t".
each tag value can get by parsing the sub string in
""
part afterkey:
similar with issue: #15893, but here: newline supporting between tags.
The text was updated successfully, but these errors were encountered: