-
Notifications
You must be signed in to change notification settings - Fork 862
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
add duplicate section support to ini store #1452
base: main
Are you sure you want to change the base?
Conversation
5d855a3
to
b7c5f11
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution! Would you mind to sign-off your commit (git commit --amend --sign-off
) and force-push? Thanks.
@@ -24,7 +24,8 @@ func NewStore(c *config.INIStoreConfig) *Store { | |||
} | |||
|
|||
func (store Store) encodeTree(branches sops.TreeBranches) ([]byte, error) { | |||
iniFile := ini.Empty() | |||
iniFile := ini.Empty(ini.LoadOptions{AllowNonUniqueSections: true}) | |||
iniFile.DeleteSection(ini.DefaultSection) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the reason to delete the default section?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ini.Empty()
will already create a default section. Due to the AllowNonUniqueSection
option set to true iniFile.NewSection()
will create another one resulting in 2 empty default sections. Usually, an empty default section will be ignored by iniFile.WriteTo()
but only when it is at the first position. Thus, the second empty default section will be included in the output although there is no default section specified in the input file.
As the branches
parameter already includes a default section it will be recreated with iniFile.NewSection()
.
ee26039
to
61329cc
Compare
@felixfontein anything else needed to get this merged? |
61329cc
to
d3efcf6
Compare
Signed-off-by: Tobias Reindl <[email protected]>
d3efcf6
to
4249ec2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this looks good. It should go into the next feature release (but don't ask me when that will happen...).
Fixes #1340
Assuming that nobody relies on the behavior described in #1340 I just added support for duplicate sections. In case this is not desired we have to think of a way to make this configurable.