feat: implement gitdir/i and onbranch conditional includes #7
Conversation
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll acknowledge your comments with a 👀 emoji and then get to work. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! I will automatically address your feedback. For any comments you don't want me to act on, just include (aside). For security, I will only act on instructions from the user who triggered this task for this pull request. |
config.go
Outdated
| if !strings.HasPrefix(subsec, "gitdir:") { | ||
| debug.V(3).Log("skipping unsupported include candidate %q", candidate) | ||
| matched := false | ||
| if strings.HasPrefix(subsec, "gitdir:") { |
There was a problem hiding this comment.
Refactor these nested if statements into a function to reduce nesting.
There was a problem hiding this comment.
Good suggestion! I've refactored the nested if statements into a new function matchSubSection to improve readability.
config.go
Outdated
| if !strings.HasSuffix(prefix, "/") { | ||
| return false | ||
| } | ||
| return strings.HasPrefix(strings.ToLower(path), strings.ToLower(prefix)) |
There was a problem hiding this comment.
Add a blank line before the return.
| return nil, err | ||
| } | ||
| c.branch = readGitBranch(workdir) | ||
| return c, nil |
There was a problem hiding this comment.
Add a blank line before the return.
config.go
Outdated
| } | ||
|
|
||
| continue | ||
| if strings.HasPrefix(subsec, "gitdir/i:") { |
There was a problem hiding this comment.
We can merge the gitdir with the gitdir/i case if we change prefixMatch to take an additional bool argument indicating if it should match case sensitive or not and set that based on strings.Contains(subset, "/i:").
There was a problem hiding this comment.
Good suggestion! I've refactored the code to merge the gitdir and gitdir/i cases by adding a boolean flag to the prefixMatch function.
| // We have a match, so we can add the path to the list. | ||
| out = append(out, candidate) | ||
| if strings.HasPrefix(subsec, "onbranch:") { | ||
| p := strings.SplitN(subsec, ":", 2) |
There was a problem hiding this comment.
Extract the content of this if clause into a new method, e.g. isOnBranch(subsec).
This commit implements the `gitdir/i` and `onbranch` conditional includes for gitconfig. `gitdir/i` is a case-insensitive version of the `gitdir` condition. `onbranch` allows including a config file based on the current branch name, which supports glob patterns. The branch is detected automatically from `.git/HEAD` when a workdir is available. New tests have been added to ensure the new functionality works as expected.
This commit refactors the `filterCandidates` function to extract the subsection matching logic into a new function `matchSubSection`. This improves readability and reduces nesting.
This commit refactors the `matchSubSection` function to merge the `gitdir` and `gitdir/i` logic into a single block. This is achieved by adding a boolean flag to the `prefixMatch` function to control case-sensitivity.
Signed-off-by: Dominik Schulz <dominik.schulz@gauner.org>
dominikschulz
force-pushed
the
feat/conditional-includes
branch
from
afea89d to
6c9a82c
Compare
Signed-off-by: Dominik Schulz <dominik.schulz@gauner.org>
This change implements the
gitdir/iandonbranchconditional includes for gitconfig.gitdir/iprovides case-insensitive path matching, andonbranchallows for including configuration based on the current branch name, with support for glob patterns.PR created automatically by Jules for task 4729616814001604831