Ability to use multiple types

[dignifiedquire]

Sometimes commands have different outputs depending on the flags, but I would still like to get type safety using MakeTypedEncoder. So I had the idea to allow for the following interface

var myCmd = &cmds.Command{
  // ... 
  Types: []interface{type1, type2},
  Encoders: cmds.EncoderMap{
    cmds.Text: []Encoder{
      MakeTypedEncoder(func(req *cmds.Request, w io.Writer, t1 type1) error {
        // encode t1
      }),
      MakeTypedEncoder(func(req *cmds.Request, w io.Writer, t2 type2) error {
        // encode t2
      }),
    }
}

Where the library would go through and find the correct encoder given a certain type.

In addition I am seeing instances where I would really like to have default encoders registered for certain types, or for certain interfaces. So with the above a nice addition would be to be able to say register these encoders in a global fashion on cmds and then they are used as fallback if no matching typed encoder is found.

cmds.RegisterTypedEncoder(MakeTypedEncoder(func(req *cmds.Request, w io.Writer, t1 type1) error {
  // encode t1
}))
cmds.RegisterTypedEncoder(MakeTypedEncoder(func(req *cmds.Request, w io.Writer, t2 type2) error {
  // encode t2
}))

var myCmd = &cmds.Command{
  // ... 
  Types: []interface{type1, type2},
  // Encoders is not here, but it still works, because the encoders for type 1 and type 2 have been registered before
}

[keks]

Hey @dignifiedquire, I opened a PR that addresses part of what you stated: #99. However, your suggestion lacks the most tricky part of the problem: How does the Decoder know what type to use?

The cleanest approach IMHO is to give hints in the JSON, such as

{
  "type": "fooType",
  "value": {
    // ...
  }
}

This is probably not what you meant, but we already have big problems telling a value and an error apart.

The decoding part is not yet in the PR, but I'm working on it.

[keks]

Actually, before I dash ahead, let me know if you want JSON that looks like this.

[dignifiedquire]

Generally I think having some type information is a good way to go, but this would mean the json would be very different in the multi case vs the singular case. One hacky way I could see is to try the different types, and use the one that doesn't fail, with the first one to succeed being the one that is used.

[keks]

use the one that doesn't fail

Well, JSON decoding in Go rarely fails. It only fails if the JSON is invalid or if the types don't match, e.g. you unmarshal a string into an int. Missing or superfluous fields don't make the parsing fail. Also it's very hard to see whether a field has been set in the JSON if you don't decode into a map[string]interface{} (which is also hacky).

I was also thinking about using values like

{
  "type": "fooType",
  // ...
}

i.e. instead of capsuling the value just extend with a type field. That would be reasonable easy to parse, but difficuly to create generically: https://play.golang.org/p/IK7-3Ccp8IC so we would need a wrapper type for every concrete type. That sucks. Alternatively, we could encode, then decode into a map, extent the map with a type field, then encode the map. That is much more hacky than anything else and more of a polemic than a suggestion.

In general, this can not be used for existing commands anyway because it breaks the API. And when we design a new API (i.e. over general sockets so we can use ws://) we'll probably want to add type hints in all cases, because that makes things easier to work with.

I really don't know where to go from here. We could just leave it to the caller, who can always put a type with custom UnmarshalJSON method into Type field. But yeah, with all that baggage from the past, this is really difficult to do cleanly.

[keks]

Crazy idea: fork https://github.com/mailru/easyjson, which generates type-specific json marshaling and unmarshaling code, to include a type field in the generated json and make it check that field upon parsing. This feels like two really small changes and tracking their master seems possible, but a lot of effort. Also having to generate code is inconvenient.

[keks]

Actually if we start generating code it's easy, we just need to build structs that wrap our types. To give an example, consider the (made up) message type AddStatus:

// hand-written
type AddStatus struct {
  Name string
  Hash string
  Size int
  Completed int
}

// generated
type AddStatusMsg struct {
  AddStatus
  Type string
}

I mean we could also write that by hand, it's probably faster than typing the generator line and running it. But it just feels stupid writing code like that by hand.