payload: {"action":"created","issue":{"url":"https://api.github.com/repos/golang/protobuf/issues/850","repository_url":"https://api.github.com/repos/golang/protobuf","labels_url":"https://api.github.com/repos/golang/protobuf/issues/850/labels{/name}","comments_url":"https://api.github.com/repos/golang/protobuf/issues/850/comments","events_url":"https://api.github.com/repos/golang/protobuf/issues/850/events","html_url":"https://github.com/golang/protobuf/issues/850","id":444086561,"node_id":"MDU6SXNzdWU0NDQwODY1NjE=","number":850,"title":"APIv2: text marshal API","user":{"login":"neild","id":52544,"node_id":"MDQ6VXNlcjUyNTQ0","avatar_url":"https://avatars1.githubusercontent.com/u/52544?v=4","gravatar_id":"","url":"https://api.github.com/users/neild","html_url":"https://github.com/neild","followers_url":"https://api.github.com/users/neild/followers","following_url":"https://api.github.com/users/neild/following{/other_user}","gists_url":"https://api.github.com/users/neild/gists{/gist_id}","starred_url":"https://api.github.com/users/neild/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/neild/subscriptions","organizations_url":"https://api.github.com/users/neild/orgs","repos_url":"https://api.github.com/users/neild/repos","events_url":"https://api.github.com/users/neild/events{/privacy}","received_events_url":"https://api.github.com/users/neild/received_events","type":"User","site_admin":false},"labels":[{"id":1439349561,"node_id":"MDU6TGFiZWwxNDM5MzQ5NTYx","url":"https://api.github.com/repos/golang/protobuf/labels/blocks-v2","name":"blocks-v2","color":"8e042b","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":3,"created_at":"2019-05-14T19:31:07Z","updated_at":"2019-07-17T16:54:04Z","closed_at":null,"author_association":"CONTRIBUTOR","body":"The v1 API provides the following functions for text (un)marshaling:\r\n\r\n```go\r\nfunc MarshalText(w io.Writer, pb protoiface.MessageV1) error {}\r\nfunc MarshalTextString(pb protoiface.MessageV1) string {}\r\nfunc UnmarshalText(s string, pb protoiface.MessageV1) error {}\r\n\r\nfunc (*TextMarshaler) Marshal(w io.Writer, pb protoiface.MessageV1) error {}\r\nfunc (*TextMarshaler) Text(pb protoiface.MessageV1) string {}\r\n```\r\n\r\nThe new API currently contains:\r\n\r\n```go\r\nfunc Marshal(m protoreflect.ProtoMessage) ([]byte, error) {}\r\nfunc Unmarshal(m protoreflect.ProtoMessage, b []byte) error {}\r\n\r\nfunc (MarshalOptions) Marshal(m protoreflect.ProtoMessage) ([]byte, error) {}\r\nfunc (UnmarshalOptions) Unmarshal(m protoreflect.ProtoMessage, b []byte) error {}\r\n```\r\n\r\nI think there are some places where the ergonomics of the new API could be improved:\r\n\r\n* There should be an easy way to convert a message into a string. Compare:\r\n\r\n   ```go\r\n   fmt.Println(proto.MarshalTextString(m))\r\n   ```\r\n\r\n   ```go\r\n   b, _ := textpb.Marshal(m) // And what do I do if there's an error?\r\n   fmt.Println(string(b))\r\n   ```\r\n\r\n* There should be a default indentation level for multi-line output. Currently, the default is compact output, with multi-line output specified by passing an indentation string in via `MarshalOptions`. Since the user is not only allowed but required to choose their own indentation level, this will result in a lot of inconsistent output. I think we should default to multi-line output (which is vastly more readable for messages of any size) with a default indentation level, with compact output opted into via a `Compact` bool on `MarshalOptions`.\r\n\r\n* `Unmarshal`'s first parameter is the message to marshal into. While this does follow the usual Go parameter ordering of (destination, source), it is inconsistent with other every other function named `Unmarshal` I can find:\r\n    * [`asn1.Unmarshal`](https://godoc.org/encoding/asn1#Unmarshal)\r\n    * [`json.Unmarshal`](https://godoc.org/encoding/json#Unmarshal)\r\n    * [`xml.Unmarshal`](https://godoc.org/encoding/xml#Unmarshal)\r\n    * [`proto.UnmarshalText`](https://godoc.org/github.com/golang/protobuf/proto#UnmarshalText)"},"comment":{"url":"https://api.github.com/repos/golang/protobuf/issues/comments/512364686","html_url":"https://github.com/golang/protobuf/issues/850#issuecomment-512364686","issue_url":"https://api.github.com/repos/golang/protobuf/issues/850","id":512364686,"node_id":"MDEyOklzc3VlQ29tbWVudDUxMjM2NDY4Ng==","user":{"login":"neild","id":52544,"node_id":"MDQ6VXNlcjUyNTQ0","avatar_url":"https://avatars1.githubusercontent.com/u/52544?v=4","gravatar_id":"","url":"https://api.github.com/users/neild","html_url":"https://github.com/neild","followers_url":"https://api.github.com/users/neild/followers","following_url":"https://api.github.com/users/neild/following{/other_user}","gists_url":"https://api.github.com/users/neild/gists{/gist_id}","starred_url":"https://api.github.com/users/neild/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/neild/subscriptions","organizations_url":"https://api.github.com/users/neild/orgs","repos_url":"https://api.github.com/users/neild/repos","events_url":"https://api.github.com/users/neild/events{/privacy}","received_events_url":"https://api.github.com/users/neild/received_events","type":"User","site_admin":false},"created_at":"2019-07-17T16:54:04Z","updated_at":"2019-07-17T16:54:04Z","author_association":"CONTRIBUTOR","body":"I think the `String` method is a distraction; perhaps we should change it, but I think it's reasonable to want to convert a message to a string using `prototext` rather than relying on the specific message's `String` method. e.g., I should be able to take an old generated message or a message from some other implementation and get text output.\r\n\r\nI propose:\r\n\r\n```go\r\npackage prototext\r\n// MarshalString returns the given proto.Message in textproto format.\r\nfunc MarshalString(m proto.Message) string {\r\n  b, _ := MarshalOptions{AllowPartial: true}.Marshal(m)\r\n  return string(b)\r\n}\r\n```\r\n\r\nAnother question is indentation. I feel we should have a default indentation level for multi-line output, to encourage consistency. I propose:\r\n\r\n```go\r\npackage prototext\r\ntype MarshalOptions struct {\r\n  // If Compact is true, messages are formatted on a single line.\r\n  Compact bool\r\n\r\n  // If Indent is a non-empty string and Compact is false, entries for a Message are\r\n  // preceded by the indent and trailed by a newline. Indent can only be\r\n  // composed of space or tab characters.\r\n  //\r\n  // If Indent is the empty string and Compact is false, a default indentation is used.\r\n  Indent string\r\n}\r\n```\r\n\r\nThe `protojson` package also has support for indentation. We could either add a `Compact` bool to `protojson.MarshalOptions` for consistency with `prototext` (indent by default) or leave it as-is for consistency with `encoding/json` (no indentation by default). I have no particular opinion on which is better."}}
other: {"actor":{"display_login":"neild"}}