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

add duplicate section support to ini store #1452

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

reindlt
Copy link

@reindlt reindlt commented Mar 7, 2024

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.

@reindlt reindlt force-pushed the ini-store-duplicate-sections branch from 5d855a3 to b7c5f11 Compare March 7, 2024 17:33
Copy link
Contributor

@felixfontein felixfontein left a 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)
Copy link
Contributor

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?

Copy link
Author

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().

@reindlt reindlt force-pushed the ini-store-duplicate-sections branch from b7c5f11 to ee26039 Compare March 11, 2024 07:34
@reindlt reindlt force-pushed the ini-store-duplicate-sections branch from ee26039 to 61329cc Compare March 11, 2024 07:49
@reindlt
Copy link
Author

reindlt commented May 3, 2024

@felixfontein anything else needed to get this merged?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ini: Encode duplicate sections
2 participants