200sc / bebop

bebop wire format in Go
Apache License 2.0
69 stars 5 forks source link

Support optionally writing comments as struct field tags in bebobc-go #14

Closed acuozzo closed 2 years ago

acuozzo commented 2 years ago

Can you please add an option to bebobc-go to write out comments as struct field tags?

I did this with my local copy by modifying writeFieldDefinition in gen.go; commenting out line 539 and adding fd.Comment to line 552 with the appropriate format string change.

I can submit a pull request, if needed. Thanks!

200sc commented 2 years ago

Being able to define tags in a nicer way than comments is probably necessary if bebop would fill the same space as protobuf.

However, allowing a well-formed comment to act as tags as a stopgap until fields can have arbitrary typed tags, with a compiler option to enable it, sounds reasonable.

Feel free to open a PR if you'd like, otherwise I can take this on as well ~this weekend.

200sc commented 2 years ago

Thinking about it more, I'd prefer to introduce some meaningful syntax instead of putting meaningful content in comments, even as a stopgap. (still controlled by a compiler flag)

Thinking about options:

Protobuf style:

struct Foo {
    int32 Example [json_name="example"]; 
}

Something closer to bebop's style:

struct Foo {
    [tag(json="example,omitempty")]
    [tag(db="ex")]
    int32 Example; 
}
200sc commented 2 years ago

Although, for compatibility with the upstream compiler, we'd need to accept those as comments:

struct Foo {
    int32 Example; // [json_name="example"]
}

struct Foo {
    //[tag(json="example,omitempty")]
    //[tag(db="ex")]
    int32 Example; 
}

And the protobuf example would be a little more painful to parse, as we'd need to finish parsing the struct then go back and modify it with the comment contents.

acuozzo commented 2 years ago

I'm sorry for the delay in replying; hectic week!

What you propose is definitely the best way to handle this. Are you still interested in a stop-gap PR from me?

200sc commented 2 years ago

@acuozzo How does PR #16 do? Happy to adjust it.

acuozzo commented 2 years ago

This works for me. Thank you!

acuozzo commented 2 years ago

I've been using this for a while now and it has been great. Thank you!

With that being said, I've recently run into an issue. The popular validator package from go-playground uses "=" in its sub-language and this seems to be interfering with PR #16.

Here's one of their examples:

type User struct {
    FirstName      string     `validate:"required"`
    LastName       string     `validate:"required"`
    Age            uint8      `validate:"gte=0,lte=130"`
    Email          string     `validate:"required,email"`
    FavouriteColor string     `validate:"iscolor"`                // alias for 'hexcolor|rgb|rgba|hsl|hsla'
    Addresses      []*Address `validate:"required,dive,required"` // a person can have a home and cottage...
}

I'd like to do something like the following in my *.bop file(s):

struct DatabaseConfiguration {
    //[tag(validate="gt=13,printascii,startswith=mongodb,uri")]
    string databaseURI;
    //[tag(validate="gt=0,lt=64,alphanum")]
    string databaseName;
}

Can we change the top-level separator from "=" to ":"?

Thanks again, –Anthony

200sc commented 2 years ago

Good point. I'm up for changing it; I'll probably get on making the change later this week, but I can also review a PR if someone gets to it before me.

200sc commented 2 years ago

@acuozzo Released with v0.2.3, off to you to close if it works, or send back if there's a problem (its fairly lightly tested).

acuozzo commented 2 years ago

@200sc This works for me. Thanks!