From 890d0ad0e1ac68c5b6b155e6837f423df883747d Mon Sep 17 00:00:00 2001 From: lnu Date: Wed, 30 Dec 2020 22:01:39 +0100 Subject: [PATCH] feat: cache compiled regex use mustcompile for regex. Always use mustcompile which throws a panic if the regex cannot be compiled. We call recover to detect the panic and display an error message to the user. --- src/environment_windows_win32.go | 18 +++++--------- src/init/omp.ps1 | 2 +- src/regex.go | 40 ++++++++++++++++++++++++++++---- src/segment.go | 18 ++++++++++---- src/segment_test.go | 20 ++++++++++++---- 5 files changed, 72 insertions(+), 26 deletions(-) diff --git a/src/environment_windows_win32.go b/src/environment_windows_win32.go index 4ffaae98..f42a8477 100644 --- a/src/environment_windows_win32.go +++ b/src/environment_windows_win32.go @@ -4,7 +4,6 @@ package main import ( "fmt" - "regexp" "strings" "syscall" "unsafe" @@ -48,10 +47,8 @@ func getWindowTitle(imageName, windowTitleRegex string) (string, error) { } // returns the first window of the first pid - _, windowTitle, err := GetWindowTitle(processPid[0], windowTitleRegex) - if err != nil { - return "", nil - } + _, windowTitle := GetWindowTitle(processPid[0], windowTitleRegex) + return windowTitle, nil } @@ -147,13 +144,10 @@ func GetWindowText(hwnd syscall.Handle, str *uint16, maxCount int32) (length int } // GetWindowTitle searches for a window attached to the pid -func GetWindowTitle(pid int, windowTitleRegex string) (syscall.Handle, string, error) { +func GetWindowTitle(pid int, windowTitleRegex string) (syscall.Handle, string) { var hwnd syscall.Handle var title string - compiledRegex, err := regexp.Compile(windowTitleRegex) - if err != nil { - return 0, "", fmt.Errorf("Error while compiling the regex '%s'", windowTitleRegex) - } + // callback fro EnumWindows cb := syscall.NewCallback(func(h syscall.Handle, p uintptr) uintptr { var prcsID int = 0 @@ -168,7 +162,7 @@ func GetWindowTitle(pid int, windowTitleRegex string) (syscall.Handle, string, e return 1 // continue enumeration } title = syscall.UTF16ToString(b) - if compiledRegex.MatchString(title) { + if matchString(windowTitleRegex, title) { // will cause EnumWindows to return 0 (error) // but we don't want to enumerate all windows since we got what we want hwnd = h @@ -183,5 +177,5 @@ func GetWindowTitle(pid int, windowTitleRegex string) (syscall.Handle, string, e // it returns 0(error occurred) instead of 1(success) // In our case, title will equal "" or the title of the window anyway _ = EnumWindows(cb, 0) - return hwnd, title, nil + return hwnd, title } diff --git a/src/init/omp.ps1 b/src/init/omp.ps1 index 5bcc1d3a..f9a9ffbb 100644 --- a/src/init/omp.ps1 +++ b/src/init/omp.ps1 @@ -45,7 +45,7 @@ function global:Set-PoshGitStatus { $config = $global:PoshSettings.Theme $cleanPWD = $PWD.ProviderPath.TrimEnd("\") $cleanPSWD = $PWD.ToString().TrimEnd("\") - $standardOut = @(&$omp --error="$errorCode" --pwd="$cleanPWD" --pswd="$cleanPSWD" --execution-time="$executionTime" --config="$config") + $standardOut = @(&$omp --error="$errorCode" --pwd="$cleanPWD" --pswd="$cleanPSWD" --execution-time="$executionTime" --config="$config" 2>&1) # Restore initial encoding [Console]::OutputEncoding = $originalOutputEncoding # the ouput can be multiline, joining these ensures proper rendering by adding line breaks with `n diff --git a/src/regex.go b/src/regex.go index 8dac1320..c12226ce 100644 --- a/src/regex.go +++ b/src/regex.go @@ -1,9 +1,36 @@ package main -import "regexp" +import ( + "regexp" + "sync" +) + +var ( + regexCache map[string]*regexp.Regexp = make(map[string]*regexp.Regexp) + regexCacheLock = sync.Mutex{} +) + +func getCompiledRegex(pattern string) *regexp.Regexp { + // try in cache first + re := regexCache[pattern] + if re != nil { + return re + } + + // should we panic or return the error? + re = regexp.MustCompile(pattern) + + // lock for concurrent access and save the compiled expression in cache + regexCacheLock.Lock() + regexCache[pattern] = re + regexCacheLock.Unlock() + + return re +} func findNamedRegexMatch(pattern, text string) map[string]string { - re := regexp.MustCompile(pattern) + // error ignored because mustCompile will cause a panic + re := getCompiledRegex(pattern) match := re.FindStringSubmatch(text) result := make(map[string]string) if len(match) == 0 { @@ -19,7 +46,7 @@ func findNamedRegexMatch(pattern, text string) map[string]string { } func findAllNamedRegexMatch(pattern, text string) []map[string]string { - re := regexp.MustCompile(pattern) + re := getCompiledRegex(pattern) match := re.FindAllStringSubmatch(text, -1) var results []map[string]string if len(match) == 0 { @@ -40,6 +67,11 @@ func findAllNamedRegexMatch(pattern, text string) []map[string]string { } func replaceAllString(pattern, text, replaceText string) string { - re := regexp.MustCompile(pattern) + re := getCompiledRegex(pattern) return re.ReplaceAllString(text, replaceText) } + +func matchString(pattern, text string) bool { + re := getCompiledRegex(pattern) + return re.MatchString(text) +} diff --git a/src/segment.go b/src/segment.go index 8174ce89..b3012a6b 100644 --- a/src/segment.go +++ b/src/segment.go @@ -3,7 +3,6 @@ package main import ( "errors" "fmt" - "regexp" ) // Segment represent a single segment and it's configuration @@ -112,10 +111,8 @@ func (segment *Segment) shouldIgnoreFolder(cwd string) bool { list := parseStringArray(value) for _, element := range list { pattern := fmt.Sprintf("^%s$", element) - matched, err := regexp.MatchString(pattern, cwd) - if err == nil && matched { - return true - } + matched := matchString(pattern, cwd) + return matched } return false } @@ -163,6 +160,17 @@ func (segment *Segment) mapSegmentWithWriter(env environmentInfo) error { } func (segment *Segment) setStringValue(env environmentInfo, cwd string) { + defer func() { + err := recover() + if err == nil { + return + } + // display a message explaining omp failed(with the err) + message := fmt.Sprintf("oh-my-posh fatal error rendering %s segment:%s", segment.Type, err) + fmt.Println(message) + segment.stringValue = "error" + segment.active = true + }() err := segment.mapSegmentWithWriter(env) if err != nil || segment.shouldIgnoreFolder(cwd) { return diff --git a/src/segment_test.go b/src/segment_test.go index da3cf752..07e4e10b 100644 --- a/src/segment_test.go +++ b/src/segment_test.go @@ -84,8 +84,14 @@ func TestShouldIgnoreFolderRegexInverted(t *testing.T) { IgnoreFolders: []string{"(?!Projects[\\/]).*"}, }, } - got := segment.shouldIgnoreFolder(cwd) - assert.False(t, got) + // detect panic(thrown by MustCompile) + defer func() { + if err := recover(); err != nil { + // display a message explaining omp failed(with the err) + assert.Equal(t, "regexp: Compile(`^(?!Projects[\\/]).*$`): error parsing regexp: invalid or unsupported Perl syntax: `(?!`", err) + } + }() + segment.shouldIgnoreFolder(cwd) } func TestShouldIgnoreFolderRegexInvertedNonEscaped(t *testing.T) { @@ -94,6 +100,12 @@ func TestShouldIgnoreFolderRegexInvertedNonEscaped(t *testing.T) { IgnoreFolders: []string{"(?!Projects/).*"}, }, } - got := segment.shouldIgnoreFolder(cwd) - assert.False(t, got) + // detect panic(thrown by MustCompile) + defer func() { + if err := recover(); err != nil { + // display a message explaining omp failed(with the err) + assert.Equal(t, "regexp: Compile(`^(?!Projects/).*$`): error parsing regexp: invalid or unsupported Perl syntax: `(?!`", err) + } + }() + segment.shouldIgnoreFolder(cwd) }