go-toml icon indicating copy to clipboard operation
go-toml copied to clipboard

Questionable behavior of 'omitempty' tag

Open Felixoid opened this issue 2 years ago • 5 comments

Describe the bug Hello. I see the current behavior of omitempty flag as a bit confusing because of a few factors:

  1. It creates empty lines at the place records should be
  2. For the slices of structures it's inherited and changes existing tags of embedded structures

To Reproduce

package main

import (
	"bytes"
	"fmt"

	"github.com/pelletier/go-toml/v2"
)

type General struct {
	From      string `toml:"from,omitempty" json:"from,omitempty" comment:"from in graphite-web format, the local TZ is used"`
	Randomize bool   `toml:"randomize" json:"randomize" comment:"randomize starting time with [0,step)"`
}

type Custom struct {
	Name string `toml:"name" json:"name,omitempty" comment:"names for generator, braces are expanded like in shell"`
	Type string `toml:"type" json:"type,omitempty" comment:"type of generator"`
	General
}
type Config struct {
	General
	Custom []Custom `toml:"custom,omitempty" json:"custom,omitempty" comment:"generators with custom parameters can be specified separately"`
}

func main() {
	buf := new(bytes.Buffer)
	config := &Config{General: General{From: "-2d", Randomize: true}}
	config.Custom = []Custom{{Name: "omit", General: General{Randomize: false}}}
	config.Custom = append(config.Custom, Custom{Name: "present", General: General{From: "-2d", Randomize: true}})
	encoder := toml.NewEncoder(buf)
	encoder.Encode(config)
	fmt.Print(buf.String())
}

it produces (includes empty line on the end)

# from in graphite-web format, the local TZ is used
from = '-2d'
# randomize starting time with [0,step)
randomize = true
# generators with custom parameters can be specified separately
[[custom]]
# names for generator, braces are expanded like in shell
name = 'omit'



[[custom]]
# names for generator, braces are expanded like in shell
name = 'present'

# from in graphite-web format, the local TZ is used
from = '-2d'
# randomize starting time with [0,step)
randomize = true

Expected behavior I expect another result:

# from in graphite-web format, the local TZ is used
from = '-2d'
# randomize starting time with [0,step)
randomize = true
# generators with custom parameters can be specified separately
[[custom]]
# names for generator, braces are expanded like in shell
name = 'omit'
# randomize starting time with [0,step)
randomize = false
[[custom]]
# names for generator, braces are expanded like in shell
name = 'present'
# from in graphite-web format, the local TZ is used
from = '-2d'
# randomize starting time with [0,step)
randomize = true

Versions

  • go-toml: version from go.sum: github.com/pelletier/go-toml/v2 v2.0.1 h1:8e3L2cCQzLFi2CR4g7vGFuFxX7Jl1kKX8gW+iV0GUKU=
  • go: go1.18.3 linux/amd64
  • operating system: linux

Additional context Add any other context about the problem here that you think may help to diagnose.

Felixoid avatar Jun 03 '22 14:06 Felixoid

Not sure if it's worth creating a new issue, so I'll ask here. I can't set alignment for toml lists in the encoder. SetIndentSymbol doesn't affect it now. Is it expected behavior?

Felixoid avatar Jun 03 '22 14:06 Felixoid

Thanks for the bug report!

It creates empty lines at the place records should be

You're right, those extra new lines shouldn't be emitted.

For the slices of structures it's inherited and changes existing tags of embedded structures

I am not sure I understand the problem. Can you share an example?

SetIndentSymbol doesn't affect it now. Is it expected behavior?

Do you have some code to reproduce the issue?

pelletier avatar Jun 08 '22 17:06 pelletier

I am not sure I understand the problem. Can you share an example?

Yes, see the first [custom] section

# given
[[custom]]
# names for generator, braces are expanded like in shell
name = 'omit'


# expected
[[custom]]
# names for generator, braces are expanded like in shell
name = 'omit'
# randomize starting time with [0,step)
randomize = false

Randomize bool `toml:"randomize" doesn't contain omitempty, but it's omitted in the output.

Do you have some code to reproduce the issue?

yes, if I use encoder := toml.NewEncoder(buf).SetIndentSymbol(" ") it does nothing with the output. I'd like to have [[custom]] content aligned.

[[custom]]
  # names for generator, braces are expanded like in shell
  name = 'omit'
  # randomize starting time with [0,step)
  randomize = false

Felixoid avatar Jun 13 '22 11:06 Felixoid

Hi, also having this issue. It looks like go-toml is creating new lines when omit empty is defined, instead of discarding the lines.

Also, I expect objects like in the example below to be omitted when all the keys in the object are omitted.

type Dependencies struct {
	Dependencies         []string `toml:"dependencies,multiline,omitempty"`
	BuildDependencies    []string `toml:"buildDependencies,multiline,omitempty"`
	OptionalDependencies []string `toml:"optionalDependencies,multiline,omitempty"`
}

type Test struct {
	Dependencies Dependencies `toml:"dependencies,omitempty"
}

Result: (notice empty lines)

[dependencies]



Expected result would be an empty file.

stingalleman avatar Jun 22 '22 18:06 stingalleman

@Felixoid for the indentation issue, you also need to .SetIndentTables(true) for the encoder to indent tables (and use whatever indentation string was provided to SetIndentSymbol).

I started working on those extra newlines and omitempty issues. It's going to take a bit because most tests were using equalStringsIgnoreNewlines, which effectively hid those issues :(

pelletier avatar Jul 16 '22 19:07 pelletier

@Felixoid @stingalleman Pull requests #803 and #798 have been merged. Unless I missed something those two should fix both the extra new lines and the incorrect propagation of omitempty. Let me know if it works!

pelletier avatar Aug 15 '22 15:08 pelletier

Hello, I didn't come right after the fix. I've had a corona, and then just everything went south ¯\_(ツ)_/¯

The output now is correct in regard to the omitempty, but SetIndentTables(true) doesn't trigger the desired behavior.

Should I create a separate issue, or could we discuss it here?

Felixoid avatar Aug 22 '23 21:08 Felixoid

@Felixoid no worries! Thank you for letting me know. I went ahead and filed a new issue https://github.com/pelletier/go-toml/issues/888

pelletier avatar Aug 25 '23 17:08 pelletier