Skip to content

Conversation

sergey-suslov
Copy link
Contributor

feature for #2458

RELEASE_NOTES=[FEATURE] Add support for gitconfig include

feature for gopasspw#2458

RELEASE_NOTES=[FEATURE] Add support for gitconfig include

Signed-off-by: Sergei Suslov <sergey.suslov.nsk@gmail.com>
@sergey-suslov sergey-suslov changed the title add gitconfig extension from include Add gitconfig include support Mar 18, 2023
RELEASE_NOTES=n/a

Signed-off-by: Sergei Suslov <sergey.suslov.nsk@gmail.com>
}
c.path = fn

loadedConfigs := map[string]struct{}{}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can initialize and add fn directly.

loadedConfigs := map[string]struct{}{}
loadedConfigs[fn] = struct{}{}
configsToLoad := []string{}
includePaths, includeExists := c.GetAll("include.path")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are you doing here? Maybe add a comment.

if includeExists {
configsToLoad = append(configsToLoad, getPathsForNestedConfig(includePaths, c.path)...)
}
for len(configsToLoad) > 0 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In Go you'd return early, i.e. if there are no configs to load you should return and otherwise continue. This saves some nesting.

@@ -264,6 +265,52 @@ func TestLoadConfig(t *testing.T) {
assert.Equal(t, "false", v)
}

func TestLoadConfigWithInclude(t *testing.T) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you plan writing to nested configs?

@dominikschulz
Copy link
Member

Thank you for your contribution. I would love to see this feature merged but in the current state I can't merge it. Due to the lack of recent activity I have to close this PR. Feel free to re-open if you get to address the pending comments.

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.

2 participants