-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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 ability to configure branch color patterns #4130
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ package presentation | |
|
||
import ( | ||
"fmt" | ||
"regexp" | ||
"strings" | ||
"time" | ||
|
||
|
@@ -18,7 +19,12 @@ import ( | |
"github.com/samber/lo" | ||
) | ||
|
||
var branchPrefixColorCache = make(map[string]style.TextStyle) | ||
type colorMatcher struct { | ||
patterns map[string]style.TextStyle | ||
isRegex bool // NOTE: this value is needed only until branchColors is deprecated and only regex color patterns are used | ||
} | ||
|
||
var branchPrefixColorCache *colorMatcher | ||
|
||
func GetBranchListDisplayStrings( | ||
branches []*models.Branch, | ||
|
@@ -125,12 +131,12 @@ func getBranchDisplayStrings( | |
|
||
// GetBranchTextStyle branch color | ||
func GetBranchTextStyle(name string) style.TextStyle { | ||
branchType := strings.Split(name, "/")[0] | ||
|
||
if value, ok := branchPrefixColorCache[branchType]; ok { | ||
return value | ||
if style, ok := branchPrefixColorCache.match(name); ok { | ||
return *style | ||
} | ||
|
||
// Default colors for common branch types | ||
branchType := strings.Split(name, "/")[0] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. keeping the old default behavior There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jesseduffield How do you feel about removing this default behavior? I find it surprising and annoying. I don't want it, but it's not obvious how to disable it; previously I had to use this to disable it:
but if I want to use the new regex patterns now, I have to say
This is very non-obvious, and I feel like I shouldn't have to do that. |
||
switch branchType { | ||
case "feature": | ||
return style.FgGreen | ||
|
@@ -143,6 +149,24 @@ func GetBranchTextStyle(name string) style.TextStyle { | |
} | ||
} | ||
|
||
func (m *colorMatcher) match(name string) (*style.TextStyle, bool) { | ||
if m.isRegex { | ||
for pattern, style := range m.patterns { | ||
if matched, _ := regexp.MatchString("^"+pattern+"$", name); matched { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I find the implicit anchoring surprising, and would remove it. If users want it, they can easily add it themselves. For the record, we don't use it for commit prefix patterns either. |
||
return &style, true | ||
} | ||
} | ||
} else { | ||
// old behavior using the deprecated branchColors behavior matching on branch type | ||
branchType := strings.Split(name, "/")[0] | ||
if value, ok := m.patterns[branchType]; ok { | ||
return &value, true | ||
} | ||
} | ||
|
||
return nil, false | ||
} | ||
|
||
func BranchStatus( | ||
branch *models.Branch, | ||
itemOperation types.ItemOperation, | ||
|
@@ -189,6 +213,9 @@ func BranchStatus( | |
return result | ||
} | ||
|
||
func SetCustomBranches(customBranchColors map[string]string) { | ||
branchPrefixColorCache = utils.SetCustomColors(customBranchColors) | ||
func SetCustomBranches(customBranchColors map[string]string, isRegex bool) { | ||
branchPrefixColorCache = &colorMatcher{ | ||
patterns: utils.SetCustomColors(customBranchColors), | ||
isRegex: isRegex, | ||
} | ||
} |
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.
It doesn't seem to make a difference in practice, but I'd find it clearer to say
len(userConfig.Gui.BranchColorPatterns) > 0
.