-
-
Notifications
You must be signed in to change notification settings - Fork 522
Add gitconfig include support #2574
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 gitconfig include support #2574
Conversation
feature for gopasspw#2458 RELEASE_NOTES=[FEATURE] Add support for gitconfig include Signed-off-by: Sergei Suslov <sergey.suslov.nsk@gmail.com>
RELEASE_NOTES=n/a Signed-off-by: Sergei Suslov <sergey.suslov.nsk@gmail.com>
} | ||
c.path = fn | ||
|
||
loadedConfigs := map[string]struct{}{} |
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.
You can initialize and add fn directly.
loadedConfigs := map[string]struct{}{} | ||
loadedConfigs[fn] = struct{}{} | ||
configsToLoad := []string{} | ||
includePaths, includeExists := c.GetAll("include.path") |
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 are you doing here? Maybe add a comment.
if includeExists { | ||
configsToLoad = append(configsToLoad, getPathsForNestedConfig(includePaths, c.path)...) | ||
} | ||
for len(configsToLoad) > 0 { |
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.
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) { |
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.
How do you plan writing to nested configs?
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. |
feature for #2458
RELEASE_NOTES=[FEATURE] Add support for gitconfig include