From 4bedd33c593ab0cb750e17c42750048904fdf7fb Mon Sep 17 00:00:00 2001 From: Junegunn Choi Date: Tue, 7 May 2024 16:58:17 +0900 Subject: [PATCH] Refactor the code to remove global variables --- src/ansi.go | 2 +- src/core.go | 34 ++++++------------- src/functions.go | 35 ++++++++++++++++++++ src/reader.go | 5 +-- src/terminal.go | 78 ++++++++++++++++++-------------------------- src/terminal_test.go | 3 +- src/tokenizer.go | 4 +-- 7 files changed, 85 insertions(+), 76 deletions(-) create mode 100644 src/functions.go diff --git a/src/ansi.go b/src/ansi.go index dbe4fd66..5f7c8be4 100644 --- a/src/ansi.go +++ b/src/ansi.go @@ -312,7 +312,7 @@ func parseAnsiCode(s string, delimiter byte) (int, byte, string) { // Inlined version of strconv.Atoi() that only handles positive // integers and does not allocate on error. code := 0 - for _, ch := range sbytes(s) { + for _, ch := range stringBytes(s) { ch -= '0' if ch > 9 { return -1, delimiter, remaining diff --git a/src/core.go b/src/core.go index dbae6a69..2a07b82a 100644 --- a/src/core.go +++ b/src/core.go @@ -4,7 +4,6 @@ package fzf import ( "sync" "time" - "unsafe" "github.com/junegunn/fzf/src/util" ) @@ -18,19 +17,6 @@ Matcher -> EvtSearchFin -> Terminal (update list) Matcher -> EvtHeader -> Terminal (update header) */ -func ustring(data []byte) string { - return unsafe.String(unsafe.SliceData(data), len(data)) -} - -func sbytes(data string) []byte { - return unsafe.Slice(unsafe.StringData(data), len(data)) -} - -type quitSignal struct { - code int - err error -} - // Run starts fzf func Run(opts *Options) (int, error) { if err := postProcessOptions(opts); err != nil { @@ -62,16 +48,16 @@ func Run(opts *Options) (int, error) { if opts.Theme.Colored { ansiProcessor = func(data []byte) (util.Chars, *[]ansiOffset) { prevLineAnsiState = lineAnsiState - trimmed, offsets, newState := extractColor(ustring(data), lineAnsiState, nil) + trimmed, offsets, newState := extractColor(byteString(data), lineAnsiState, nil) lineAnsiState = newState - return util.ToChars(sbytes(trimmed)), offsets + return util.ToChars(stringBytes(trimmed)), offsets } } else { // When color is disabled but ansi option is given, // we simply strip out ANSI codes from the input ansiProcessor = func(data []byte) (util.Chars, *[]ansiOffset) { - trimmed, _, _ := extractColor(ustring(data), nil, nil) - return util.ToChars(sbytes(trimmed)), nil + trimmed, _, _ := extractColor(byteString(data), nil, nil) + return util.ToChars(stringBytes(trimmed)), nil } } } @@ -83,7 +69,7 @@ func Run(opts *Options) (int, error) { if len(opts.WithNth) == 0 { chunkList = NewChunkList(func(item *Item, data []byte) bool { if len(header) < opts.HeaderLines { - header = append(header, ustring(data)) + header = append(header, byteString(data)) eventBox.Set(EvtHeader, header) return false } @@ -94,7 +80,7 @@ func Run(opts *Options) (int, error) { }) } else { chunkList = NewChunkList(func(item *Item, data []byte) bool { - tokens := Tokenize(ustring(data), opts.Delimiter) + tokens := Tokenize(byteString(data), opts.Delimiter) if opts.Ansi && opts.Theme.Colored && len(tokens) > 1 { var ansiState *ansiState if prevLineAnsiState != nil { @@ -118,7 +104,7 @@ func Run(opts *Options) (int, error) { eventBox.Set(EvtHeader, header) return false } - item.text, item.colors = ansiProcessor(sbytes(transformed)) + item.text, item.colors = ansiProcessor(stringBytes(transformed)) item.text.TrimTrailingWhitespaces() item.text.Index = itemIndex item.origText = &data @@ -241,7 +227,7 @@ func Run(opts *Options) (int, error) { // Event coordination reading := true ticks := 0 - var nextCommand *string + var nextCommand *commandSpec var nextEnviron []string eventBox.Watch(EvtReadNew) total := 0 @@ -262,7 +248,7 @@ func Run(opts *Options) (int, error) { useSnapshot := false var snapshot []*Chunk var count int - restart := func(command string, environ []string) { + restart := func(command commandSpec, environ []string) { reading = true chunkList.Clear() itemIndex = 0 @@ -328,7 +314,7 @@ func Run(opts *Options) (int, error) { matcher.Reset(snapshot, input(), false, !reading, sort, snapshotRevision) case EvtSearchNew: - var command *string + var command *commandSpec var environ []string var changed bool switch val := value.(type) { diff --git a/src/functions.go b/src/functions.go new file mode 100644 index 00000000..f16371a2 --- /dev/null +++ b/src/functions.go @@ -0,0 +1,35 @@ +package fzf + +import ( + "os" + "strings" + "unsafe" +) + +func writeTemporaryFile(data []string, printSep string) string { + f, err := os.CreateTemp("", "fzf-preview-*") + if err != nil { + // Unable to create temporary file + // FIXME: Should we terminate the program? + return "" + } + defer f.Close() + + f.WriteString(strings.Join(data, printSep)) + f.WriteString(printSep) + return f.Name() +} + +func removeFiles(files []string) { + for _, filename := range files { + os.Remove(filename) + } +} + +func stringBytes(data string) []byte { + return unsafe.Slice(unsafe.StringData(data), len(data)) +} + +func byteString(data []byte) string { + return unsafe.String(unsafe.SliceData(data), len(data)) +} diff --git a/src/reader.go b/src/reader.go index 6092087e..99a609e2 100644 --- a/src/reader.go +++ b/src/reader.go @@ -86,11 +86,12 @@ func (r *Reader) terminate() { r.mutex.Unlock() } -func (r *Reader) restart(command string, environ []string) { +func (r *Reader) restart(command commandSpec, environ []string) { r.event = int32(EvtReady) r.startEventPoller() - success := r.readFromCommand(command, environ) + success := r.readFromCommand(command.command, environ) r.fin(success) + removeFiles(command.tempFiles) } func (r *Reader) readChannel(inputChan chan string) bool { diff --git a/src/terminal.go b/src/terminal.go index c5f718a2..6fe90cae 100644 --- a/src/terminal.go +++ b/src/terminal.go @@ -50,8 +50,6 @@ var placeholder *regexp.Regexp var whiteSuffix *regexp.Regexp var offsetComponentRegex *regexp.Regexp var offsetTrimCharsRegex *regexp.Regexp -var activeTempFiles []string -var activeTempFilesMutex sync.Mutex var passThroughRegex *regexp.Regexp const clearCode string = "\x1b[2J" @@ -64,8 +62,6 @@ func init() { whiteSuffix = regexp.MustCompile(`\s*$`) offsetComponentRegex = regexp.MustCompile(`([+-][0-9]+)|(-?/[1-9][0-9]*)`) offsetTrimCharsRegex = regexp.MustCompile(`[^0-9/+-]`) - activeTempFiles = []string{} - activeTempFilesMutex = sync.Mutex{} // Parts of the preview output that should be passed through to the terminal // * https://github.com/tmux/tmux/wiki/FAQ#what-is-the-passthrough-escape-sequence-and-how-do-i-use-it @@ -115,6 +111,16 @@ func (s *resumableState) Set(flag bool) { } } +type commandSpec struct { + command string + tempFiles []string +} + +type quitSignal struct { + code int + err error +} + type previewer struct { version int64 lines []string @@ -507,7 +513,7 @@ type placeholderFlags struct { type searchRequest struct { sort bool sync bool - command *string + command *commandSpec environ []string changed bool } @@ -2530,32 +2536,6 @@ func hasPreviewFlags(template string) (slot bool, plus bool, forceUpdate bool) { return } -func writeTemporaryFile(data []string, printSep string) string { - f, err := os.CreateTemp("", "fzf-preview-*") - if err != nil { - // Unable to create temporary file - // FIXME: Should we terminate the program? - return "" - } - defer f.Close() - - f.WriteString(strings.Join(data, printSep)) - f.WriteString(printSep) - activeTempFilesMutex.Lock() - activeTempFiles = append(activeTempFiles, f.Name()) - activeTempFilesMutex.Unlock() - return f.Name() -} - -func cleanTemporaryFiles() { - activeTempFilesMutex.Lock() - for _, filename := range activeTempFiles { - os.Remove(filename) - } - activeTempFiles = []string{} - activeTempFilesMutex.Unlock() -} - type replacePlaceholderParams struct { template string stripAnsi bool @@ -2569,7 +2549,7 @@ type replacePlaceholderParams struct { executor *util.Executor } -func (t *Terminal) replacePlaceholder(template string, forcePlus bool, input string, list []*Item) string { +func (t *Terminal) replacePlaceholder(template string, forcePlus bool, input string, list []*Item) (string, []string) { return replacePlaceholder(replacePlaceholderParams{ template: template, stripAnsi: t.ansi, @@ -2590,8 +2570,9 @@ func (t *Terminal) evaluateScrollOffset() int { } // We only need the current item to calculate the scroll offset - offsetExpr := offsetTrimCharsRegex.ReplaceAllString( - t.replacePlaceholder(t.previewOpts.scroll, false, "", []*Item{t.currentItem(), nil}), "") + replaced, tempFiles := t.replacePlaceholder(t.previewOpts.scroll, false, "", []*Item{t.currentItem(), nil}) + removeFiles(tempFiles) + offsetExpr := offsetTrimCharsRegex.ReplaceAllString(replaced, "") atoi := func(s string) int { n, e := strconv.Atoi(s) @@ -2619,7 +2600,8 @@ func (t *Terminal) evaluateScrollOffset() int { return util.Max(0, base) } -func replacePlaceholder(params replacePlaceholderParams) string { +func replacePlaceholder(params replacePlaceholderParams) (string, []string) { + tempFiles := []string{} current := params.allItems[:1] selected := params.allItems[1:] if current[0] == nil { @@ -2630,7 +2612,7 @@ func replacePlaceholder(params replacePlaceholderParams) string { } // replace placeholders one by one - return placeholder.ReplaceAllStringFunc(params.template, func(match string) string { + replaced := placeholder.ReplaceAllStringFunc(params.template, func(match string) string { escaped, match, flags := parsePlaceholder(match) // this function implements the effects a placeholder has on items @@ -2713,10 +2695,14 @@ func replacePlaceholder(params replacePlaceholderParams) string { } if flags.file { - return writeTemporaryFile(replacements, params.printsep) + file := writeTemporaryFile(replacements, params.printsep) + tempFiles = append(tempFiles, file) + return file } return strings.Join(replacements, " ") }) + + return replaced, tempFiles } func (t *Terminal) redraw() { @@ -2733,7 +2719,7 @@ func (t *Terminal) executeCommand(template string, forcePlus bool, background bo if !valid && !capture { return line } - command := t.replacePlaceholder(template, forcePlus, string(t.input), list) + command, tempFiles := t.replacePlaceholder(template, forcePlus, string(t.input), list) cmd := t.executor.ExecCommand(command, false) cmd.Env = t.environ() t.executing.Set(true) @@ -2764,7 +2750,7 @@ func (t *Terminal) executeCommand(template string, forcePlus bool, background bo } } t.executing.Set(false) - cleanTemporaryFiles() + removeFiles(tempFiles) return line } @@ -3042,7 +3028,7 @@ func (t *Terminal) Loop() error { // We don't display preview window if no match if items[0] != nil { _, query := t.Input() - command := t.replacePlaceholder(commandTemplate, false, string(query), items) + command, tempFiles := t.replacePlaceholder(commandTemplate, false, string(query), items) cmd := t.executor.ExecCommand(command, true) if pwindowSize.Lines > 0 { lines := fmt.Sprintf("LINES=%d", pwindowSize.Lines) @@ -3164,12 +3150,11 @@ func (t *Terminal) Loop() error { finishChan <- true // Tell Goroutine 3 to stop <-reapChan // Goroutine 2 and 3 finished <-reapChan + removeFiles(tempFiles) } else { // Failed to start the command. Report the error immediately. t.reqBox.Set(reqPreviewDisplay, previewResult{version, []string{err.Error()}, 0, ""}) } - - cleanTemporaryFiles() } else { t.reqBox.Set(reqPreviewDisplay, previewResult{version, nil, 0, ""}) } @@ -3343,7 +3328,7 @@ func (t *Terminal) Loop() error { pbarDragging := false wasDown := false for looping { - var newCommand *string + var newCommand *commandSpec var reloadSync bool changed := false beof := false @@ -3468,7 +3453,8 @@ func (t *Terminal) Loop() error { case actBecome: valid, list := t.buildPlusList(a.a, false) if valid { - command := t.replacePlaceholder(a.a, false, string(t.input), list) + // We do not remove temp files in this case + command, _ := t.replacePlaceholder(a.a, false, string(t.input), list) t.tui.Close() if t.history != nil { t.history.append(string(t.input)) @@ -4140,8 +4126,8 @@ func (t *Terminal) Loop() error { valid = !slot || forceUpdate } if valid { - command := t.replacePlaceholder(a.a, false, string(t.input), list) - newCommand = &command + command, tempFiles := t.replacePlaceholder(a.a, false, string(t.input), list) + newCommand = &commandSpec{command, tempFiles} reloadSync = a.t == actReloadSync t.reading = true } diff --git a/src/terminal_test.go b/src/terminal_test.go index a448de30..317cd5f9 100644 --- a/src/terminal_test.go +++ b/src/terminal_test.go @@ -13,7 +13,7 @@ import ( ) func replacePlaceholderTest(template string, stripAnsi bool, delimiter Delimiter, printsep string, forcePlus bool, query string, allItems []*Item) string { - return replacePlaceholder(replacePlaceholderParams{ + replaced, _ := replacePlaceholder(replacePlaceholderParams{ template: template, stripAnsi: stripAnsi, delimiter: delimiter, @@ -25,6 +25,7 @@ func replacePlaceholderTest(template string, stripAnsi bool, delimiter Delimiter prompt: "prompt", executor: util.NewExecutor(""), }) + return replaced } func TestReplacePlaceholder(t *testing.T) { diff --git a/src/tokenizer.go b/src/tokenizer.go index defe8cce..6b2ef72b 100644 --- a/src/tokenizer.go +++ b/src/tokenizer.go @@ -91,7 +91,7 @@ func withPrefixLengths(tokens []string, begin int) []Token { prefixLength := begin for idx := range tokens { - chars := util.ToChars(sbytes(tokens[idx])) + chars := util.ToChars(stringBytes(tokens[idx])) ret[idx] = Token{&chars, int32(prefixLength)} prefixLength += chars.Length() } @@ -187,7 +187,7 @@ func Transform(tokens []Token, withNth []Range) []Token { if r.begin == r.end { idx := r.begin if idx == rangeEllipsis { - chars := util.ToChars(sbytes(joinTokens(tokens))) + chars := util.ToChars(stringBytes(joinTokens(tokens))) parts = append(parts, &chars) } else { if idx < 0 {