Don't print struct fields with zero values by default

[wtertius]

No description provided.

[k0kubun]

We're trying to reduce the configuration variables for simplicity. On the other hand, we do not want breaking changes if possible.

First of all, you did not describe why. Could you describe on what kind of situations this behavior is beneficial? (I mean, please provide example outputs with "before" and "after" which are improved by this feature.)

[wtertius]

Sure.

There are two cases, when it's helpful for us:

• We have a lot of go types generated from WSDL. They include more than 70% of fields that are empty in the most of cases. But they still must be there.
• We use gRPC. It generates auxiliary fields. See the example below.

Yes, it silently introduces changed behaviour.
Of course, there are two possible purposes, when somebody prints value:
(1) Look at the payload data
(2) Look at the types
I introduced the change with assumption, that the (1) is more common. And for (2) usually people walk the types with IDE.

Another good idea is that it should be convenient to configure the pp/printer in one line, e. g.:

pp.Printer().Indent(4).Emitempty().Println(value)

But it's topic for another discussion.

Example

Before:

// pp.PrintZeroStructFields(true)
OutDate: &customtime.CustomTime{
    Time:                 pp.StrToTime("2019-05-06T00:00:00Z"),
    XXX_NoUnkeyedLiteral: struct {}{},
    XXX_unrecognized:     []uint8{},
    XXX_sizecache:        0,
},
InDate: &customtime.CustomTime{
	Time:                 pp.StrToTime("0001-01-01T00:00:00Z"),
	XXX_NoUnkeyedLiteral: struct {}{},
	XXX_unrecognized:     []uint8{},
	XXX_sizecache:        0,
},

After:

  OutDate: &customtime.CustomTime{
    Time: pp.StrToTime("2019-05-06T00:00:00Z"),
  },
  InDate: &customtime.CustomTime{
  }

Pay attention:

• InDate: isn't removed totally in After case to underline that it has empty value but not nil
• Time: pp.StrToTime("2019-05-06T00:00:00Z"),: It's valid go code. The exact format to be used can be discussed.

[k0kubun]

I'm okay for accepting https://github.com/k0kubun/pp/issues/22 as your example makes sense (thanks!). But,

Before:
Time: pp.StrToTime("2019-05-06T00:00:00Z"),

This was not before, right? And I can see you're modifying at least 3 things (omitempty, indent, StrToTime).

"It's valid go code", true, and a motivation to make it valid code is understandable, but at the same time StrToTime is your original function which others may be surprised at. I would like to discuss that separately and including it here would block two other things. Could you separate pull requests?

[k0kubun]

Another good idea is that it should be convenient to configure the pp/printer in one line, e. g.:
pp.Printer().Indent(4).Emitempty().Println(value)

I would like to have a separate issue for this too but in this case it's related to this feature as it requires a configuration. Maybe we need to solve that first to have this feature.

I'm fine with the method chain but on the other hand I feel it's more Java-ish. How about this?

// one line (maybe not possible with gofmt?)
pp.Printer{ Indent: 4, Emitempty: true }.Println(value)

// another use case
p := pp.Printer{ Indent: 4, Emitempty: true }
p.Println(value)

Anyway both ideas do not affect things globally and I feel they're better than this PR's current strategy.

[wtertius]

I'm okay for accepting #22 as your example makes sense (thanks!). But,

Before:
Time: pp.StrToTime("2019-05-06T00:00:00Z"),

This was not before, right? And I can see you're modifying at least 3 things (omitempty, indent, StrToTime).

"It's valid go code", true, and a motivation to make it valid code is understandable, but at the same time StrToTime is your original function which others may be surprised at. I would like to discuss that separately and including it here would block two other things. Could you separate pull requests?

Moved the code to https://github.com/k0kubun/pp/pull/31/files

[wtertius]

Another good idea is that it should be convenient to configure the pp/printer in one line, e. g.:
pp.Printer().Indent(4).Emitempty().Println(value)

I would like to have a separate issue for this too but in this case it's related to this feature as it requires a configuration. Maybe we need to solve that first to have this feature.

I'm fine with the method chain but on the other hand I feel it's more Java-ish. How about this?

// one line (maybe not possible with gofmt?)
pp.Printer{ Indent: 4, Emitempty: true }.Println(value)

// another use case
p := pp.Printer{ Indent: 4, Emitempty: true }
p.Println(value)

Anyway both ideas do not affect things globally and I feel they're better than this PR's current strategy.

Well, it's a good idea, but it requires much more efforts than only Omitempty implementation:
It's the interface and should be discussed and designed properly.

I suggest here to solve omitempty implementation. Then we can discuss the new interface. Better in chat.
What do you think?

[k0kubun]

Moved the code to https://github.com/k0kubun/pp/pull/31/files

Thanks, appreciated.

Better in chat.

Sorry, asynchronously responding to your comments is the maximum effort which I can take while I'm busy these days. Though thank you for your sincere attitude for making a good decision.

I suggest here to solve omitempty implementation. Then we can discuss the new interface.

Completely understandable. But at the same time what I do not want here are:

1. Introduce breaking changes (change omitempty to default) without allowing to use old behavior at least for a short term
2. Increase the set of global config (indentWidth, emitEmpty)

Currently this PR satisfies 1 but violates 2 (it's my fault to leave such things now), and it can't be merged anyways. So, how about separating PR for indent as well and discuss interface of that there first? After having the interface, we can easily satisfy 1 and 2 at the same time easily for this.

[k0kubun]

Closing due to inactivity and a conflict with #33.