Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Questionable behavior of 'omitempty' tag #786

Closed
Felixoid opened this issue Jun 3, 2022 · 8 comments · Fixed by #803
Closed

Questionable behavior of 'omitempty' tag #786

Felixoid opened this issue Jun 3, 2022 · 8 comments · Fixed by #803
Assignees
Labels
bug Issues describing a bug in go-toml.

Comments

@Felixoid
Copy link
Contributor

Felixoid commented Jun 3, 2022

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
Copy link
Contributor Author

Felixoid commented Jun 3, 2022

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?

@pelletier
Copy link
Owner

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 pelletier added the bug Issues describing a bug in go-toml. label Jun 8, 2022
@Felixoid
Copy link
Contributor Author

Felixoid commented Jun 13, 2022

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

@stingalleman
Copy link

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.

@pelletier
Copy link
Owner

@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
Copy link
Owner

@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!

@Felixoid
Copy link
Contributor Author

Felixoid commented Aug 22, 2023

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?

@pelletier
Copy link
Owner

@Felixoid no worries! Thank you for letting me know. I went ahead and filed a new issue #888

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issues describing a bug in go-toml.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants