From 2ae4daf3117d362c0a7d1ad2edd7988529219bf5 Mon Sep 17 00:00:00 2001 From: mitchell Date: Thu, 26 Jan 2023 16:18:27 -0500 Subject: [PATCH 1/9] Do not use terminal escape sequences in non-interactive terminals or the Windows command prompt. --- internal/output/progress.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/internal/output/progress.go b/internal/output/progress.go index 3c1e6a42c7..a72ec5a43d 100644 --- a/internal/output/progress.go +++ b/internal/output/progress.go @@ -2,8 +2,12 @@ package output import ( "fmt" + "os" + "runtime" "strings" "time" + + "golang.org/x/crypto/ssh/terminal" ) const moveCaretBack = "\x1b[%dD" // %d is the number of characters to move back @@ -24,7 +28,7 @@ func (d *Spinner) MarshalOutput(f Format) interface{} { func StartSpinner(out Outputer, msg string, interval time.Duration) *Spinner { frames := []string{".", "..", "..."} - if out.Config().Interactive { + if isInteractive(out) { frames = []string{`|`, `/`, `-`, `\`} } d := &Spinner{0, frames, out, make(chan struct{}, 1), interval} @@ -42,7 +46,7 @@ func StartSpinner(out Outputer, msg string, interval time.Duration) *Spinner { } func (d *Spinner) moveCaretBack() int { - if !d.out.Config().Interactive { + if !isInteractive(d.out) { return 0 } prevPos := d.frame - 1 @@ -96,3 +100,7 @@ func (d *Spinner) Stop(msg string) { d.out.Fprint(d.out.Config().ErrWriter, "\n") } + +func isInteractive(out Outputer) bool { + return out.Config().Interactive && terminal.IsTerminal(int(os.Stdin.Fd())) && (runtime.GOOS != "windows" || os.Getenv("SHELL") != "") +} From 2bca7c263e9480d2915400d36edd52729d56773f Mon Sep 17 00:00:00 2001 From: mitchell Date: Fri, 27 Jan 2023 11:12:53 -0500 Subject: [PATCH 2/9] Support interactive progress in cmd.exe. --- cmd/state/output.go | 7 ++--- internal/output/progress.go | 17 ++--------- internal/output/progress_unix.go | 14 +++++++++ internal/output/progress_win.go | 50 ++++++++++++++++++++++++++++++++ 4 files changed, 70 insertions(+), 18 deletions(-) create mode 100644 internal/output/progress_unix.go create mode 100644 internal/output/progress_win.go diff --git a/cmd/state/output.go b/cmd/state/output.go index d5912ae30c..cb78fafb57 100644 --- a/cmd/state/output.go +++ b/cmd/state/output.go @@ -4,15 +4,14 @@ import ( "errors" "os" - "github.com/jessevdk/go-flags" - "github.com/ActiveState/cli/internal/errs" "github.com/ActiveState/cli/internal/logging" "github.com/ActiveState/cli/internal/multilog" "github.com/ActiveState/cli/internal/output" "github.com/ActiveState/cli/internal/rollbar" "github.com/ActiveState/cli/internal/terminal" - + "github.com/jessevdk/go-flags" + "golang.org/x/term" survey "gopkg.in/AlecAivazis/survey.v1/core" ) @@ -56,7 +55,7 @@ func initOutput(flags outputFlags, formatName string) (output.Outputer, error) { OutWriter: os.Stdout, ErrWriter: os.Stderr, Colored: !flags.DisableColor(), - Interactive: true, + Interactive: term.IsTerminal(int(os.Stdin.Fd())), }) if err != nil { if errors.Is(err, output.ErrNotRecognized) { diff --git a/internal/output/progress.go b/internal/output/progress.go index a72ec5a43d..4b59ef134b 100644 --- a/internal/output/progress.go +++ b/internal/output/progress.go @@ -1,17 +1,10 @@ package output import ( - "fmt" - "os" - "runtime" "strings" "time" - - "golang.org/x/crypto/ssh/terminal" ) -const moveCaretBack = "\x1b[%dD" // %d is the number of characters to move back - type Spinner struct { frame int frames []string @@ -28,7 +21,7 @@ func (d *Spinner) MarshalOutput(f Format) interface{} { func StartSpinner(out Outputer, msg string, interval time.Duration) *Spinner { frames := []string{".", "..", "..."} - if isInteractive(out) { + if out.Config().Interactive { frames = []string{`|`, `/`, `-`, `\`} } d := &Spinner{0, frames, out, make(chan struct{}, 1), interval} @@ -46,7 +39,7 @@ func StartSpinner(out Outputer, msg string, interval time.Duration) *Spinner { } func (d *Spinner) moveCaretBack() int { - if !isInteractive(d.out) { + if !d.out.Config().Interactive { return 0 } prevPos := d.frame - 1 @@ -54,7 +47,7 @@ func (d *Spinner) moveCaretBack() int { prevPos = len(d.frames) - 1 } prevFrame := d.frames[prevPos] - d.out.Fprint(d.out.Config().ErrWriter, fmt.Sprintf(moveCaretBack, len(prevFrame))) + d.moveCaretBackInTerminal(len(prevFrame)) return len(prevFrame) } @@ -100,7 +93,3 @@ func (d *Spinner) Stop(msg string) { d.out.Fprint(d.out.Config().ErrWriter, "\n") } - -func isInteractive(out Outputer) bool { - return out.Config().Interactive && terminal.IsTerminal(int(os.Stdin.Fd())) && (runtime.GOOS != "windows" || os.Getenv("SHELL") != "") -} diff --git a/internal/output/progress_unix.go b/internal/output/progress_unix.go new file mode 100644 index 0000000000..2a719bfb13 --- /dev/null +++ b/internal/output/progress_unix.go @@ -0,0 +1,14 @@ +//go:build linux || darwin +// +build linux darwin + +package output + +import ( + "fmt" +) + +const moveCaretBack = "\x1b[%dD" // %d is the number of characters to move back + +func (d *Spinner) moveCaretBackInTerminal(n int) { + d.out.Fprint(d.out.Config().ErrWriter, fmt.Sprintf(moveCaretBack, n)) +} diff --git a/internal/output/progress_win.go b/internal/output/progress_win.go new file mode 100644 index 0000000000..f31b88e6be --- /dev/null +++ b/internal/output/progress_win.go @@ -0,0 +1,50 @@ +//go:build windows +// +build windows + +package output + +import ( + "os" + "syscall" + "unsafe" +) + +var kernel32 = syscall.NewLazyDLL("kernel32.dll") +var procGetConsoleScreenBufferInfo = kernel32.NewProc("GetConsoleScreenBufferInfo") +var procSetConsoleCursorPosition = kernel32.NewProc("SetConsoleCursorPosition") + +type coord struct { + x short + y short +} + +type short int16 +type word uint16 + +type smallRect struct { + bottom short + left short + right short + top short +} + +type consoleScreenBufferInfo struct { + size coord + cursorPosition coord + attributes word + window smallRect + maximumWindowSize coord +} + +func (d *Spinner) moveCaretBackInTerminal(n int) { + handle := syscall.Handle(os.Stdout.Fd()) + + var csbi consoleScreenBufferInfo + _, _, _ = procGetConsoleScreenBufferInfo.Call(uintptr(handle), uintptr(unsafe.Pointer(&csbi))) + + var cursor coord + cursor.x = csbi.cursorPosition.x + short(-n) + cursor.y = csbi.cursorPosition.y + + _, _, _ = procSetConsoleCursorPosition.Call(uintptr(handle), uintptr(*(*int32)(unsafe.Pointer(&cursor)))) +} From 44da2541a91033f65c8292bcc8610e8d5f230755 Mon Sep 17 00:00:00 2001 From: mitchell Date: Fri, 27 Jan 2023 13:09:27 -0500 Subject: [PATCH 3/9] Handle terminals other than cmd.exe on Windows. --- cmd/state/main.go | 2 +- cmd/state/output.go | 5 +++-- internal/output/output.go | 1 + internal/output/progress.go | 13 ++++++++++++- internal/output/progress_unix.go | 10 ++-------- internal/output/progress_win.go | 2 +- 6 files changed, 20 insertions(+), 13 deletions(-) diff --git a/cmd/state/main.go b/cmd/state/main.go index 726ca20a4c..a268a5c4e7 100644 --- a/cmd/state/main.go +++ b/cmd/state/main.go @@ -82,7 +82,7 @@ func main() { // Set up our output formatter/writer outFlags := parseOutputFlags(os.Args) - out, err := initOutput(outFlags, "") + out, err := initOutput(outFlags, "", subshell.New(cfg).Shell()) if err != nil { multilog.Critical("Could not initialize outputer: %s", errs.JoinMessage(err)) os.Stderr.WriteString(locale.Tr("err_main_outputer", err.Error())) diff --git a/cmd/state/output.go b/cmd/state/output.go index cb78fafb57..d28993e44b 100644 --- a/cmd/state/output.go +++ b/cmd/state/output.go @@ -46,7 +46,7 @@ func parseOutputFlags(args []string) outputFlags { return flagSet } -func initOutput(flags outputFlags, formatName string) (output.Outputer, error) { +func initOutput(flags outputFlags, formatName string, shellName string) (output.Outputer, error) { if formatName == "" { formatName = flags.Output } @@ -56,12 +56,13 @@ func initOutput(flags outputFlags, formatName string) (output.Outputer, error) { ErrWriter: os.Stderr, Colored: !flags.DisableColor(), Interactive: term.IsTerminal(int(os.Stdin.Fd())), + ShellName: shellName, }) if err != nil { if errors.Is(err, output.ErrNotRecognized) { // The formatter might still be registered, so default to plain for now logging.Warning("Output format not recognized: %s, defaulting to plain output instead", formatName) - return initOutput(flags, string(output.PlainFormatName)) + return initOutput(flags, string(output.PlainFormatName), shellName) } multilog.Log(logging.ErrorNoStacktrace, rollbar.Error)("Could not create outputer, name: %s, error: %s", formatName, err.Error()) return nil, errs.Wrap(err, "output.New %s failed", formatName) diff --git a/internal/output/output.go b/internal/output/output.go index 7082da6277..54c3a47359 100644 --- a/internal/output/output.go +++ b/internal/output/output.go @@ -90,4 +90,5 @@ type Config struct { ErrWriter io.Writer Colored bool Interactive bool + ShellName string } diff --git a/internal/output/progress.go b/internal/output/progress.go index 4b59ef134b..53f2565a0c 100644 --- a/internal/output/progress.go +++ b/internal/output/progress.go @@ -1,10 +1,13 @@ package output import ( + "fmt" "strings" "time" ) +const moveCaretBackEscapeSequence = "\x1b[%dD" // %d is the number of characters to move back + type Spinner struct { frame int frames []string @@ -47,7 +50,11 @@ func (d *Spinner) moveCaretBack() int { prevPos = len(d.frames) - 1 } prevFrame := d.frames[prevPos] - d.moveCaretBackInTerminal(len(prevFrame)) + if d.out.Config().ShellName != "cmd" { // cannot use subshell/cmd.Name due to import cycle + d.moveCaretBackInTerminal(len(prevFrame)) + } else { + d.moveCaretBackInCommandPrompt(len(prevFrame)) + } return len(prevFrame) } @@ -93,3 +100,7 @@ func (d *Spinner) Stop(msg string) { d.out.Fprint(d.out.Config().ErrWriter, "\n") } + +func (d *Spinner) moveCaretBackInTerminal(n int) { + d.out.Fprint(d.out.Config().ErrWriter, fmt.Sprintf(moveCaretBackEscapeSequence, n)) +} diff --git a/internal/output/progress_unix.go b/internal/output/progress_unix.go index 2a719bfb13..e1cdd640f0 100644 --- a/internal/output/progress_unix.go +++ b/internal/output/progress_unix.go @@ -3,12 +3,6 @@ package output -import ( - "fmt" -) - -const moveCaretBack = "\x1b[%dD" // %d is the number of characters to move back - -func (d *Spinner) moveCaretBackInTerminal(n int) { - d.out.Fprint(d.out.Config().ErrWriter, fmt.Sprintf(moveCaretBack, n)) +func (d *Spinner) moveCaretBackInCommandPrompt(n int) { + // No-op } diff --git a/internal/output/progress_win.go b/internal/output/progress_win.go index f31b88e6be..8416624c95 100644 --- a/internal/output/progress_win.go +++ b/internal/output/progress_win.go @@ -36,7 +36,7 @@ type consoleScreenBufferInfo struct { maximumWindowSize coord } -func (d *Spinner) moveCaretBackInTerminal(n int) { +func (d *Spinner) moveCaretBackInCommandPrompt(n int) { handle := syscall.Handle(os.Stdout.Fd()) var csbi consoleScreenBufferInfo From 261efb02bf4d003ae8a1d486c1829068044fa601 Mon Sep 17 00:00:00 2001 From: mitchell Date: Fri, 27 Jan 2023 13:25:14 -0500 Subject: [PATCH 4/9] Fixed failing unit tests. --- cmd/state/main_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/cmd/state/main_test.go b/cmd/state/main_test.go index 3dc32eac33..3efbf59c25 100644 --- a/cmd/state/main_test.go +++ b/cmd/state/main_test.go @@ -15,37 +15,37 @@ type MainTestSuite struct { func (suite *MainTestSuite) TestOutputer() { { - outputer, err := initOutput(outputFlags{"", false, false, false}, "") + outputer, err := initOutput(outputFlags{"", false, false, false}, "", "") suite.Require().NoError(err, errs.Join(err, "\n").Error()) suite.Equal(output.PlainFormatName, outputer.Type(), "Returns Plain outputer") } { - outputer, err := initOutput(outputFlags{string(output.PlainFormatName), false, false, false}, "") + outputer, err := initOutput(outputFlags{string(output.PlainFormatName), false, false, false}, "", "") suite.Require().NoError(err) suite.Equal(output.PlainFormatName, outputer.Type(), "Returns Plain outputer") } { - outputer, err := initOutput(outputFlags{string(output.JSONFormatName), false, false, false}, "") + outputer, err := initOutput(outputFlags{string(output.JSONFormatName), false, false, false}, "", "") suite.Require().NoError(err) suite.Equal(output.JSONFormatName, outputer.Type(), "Returns JSON outputer") } { - outputer, err := initOutput(outputFlags{"", false, false, false}, string(output.JSONFormatName)) + outputer, err := initOutput(outputFlags{"", false, false, false}, string(output.JSONFormatName), "") suite.Require().NoError(err) suite.Equal(output.JSONFormatName, outputer.Type(), "Returns JSON outputer") } { - outputer, err := initOutput(outputFlags{"", false, false, false}, string(output.EditorFormatName)) + outputer, err := initOutput(outputFlags{"", false, false, false}, string(output.EditorFormatName), "") suite.Require().NoError(err) suite.Equal(output.EditorFormatName, outputer.Type(), "Returns JSON outputer") } { - outputer, err := initOutput(outputFlags{"", false, false, false}, string(output.EditorV0FormatName)) + outputer, err := initOutput(outputFlags{"", false, false, false}, string(output.EditorV0FormatName), "") suite.Require().NoError(err) suite.Equal(output.EditorV0FormatName, outputer.Type(), "Returns JSON outputer") } From a0d6a0a07c10d10ef19604a238e9e4c8864de644 Mon Sep 17 00:00:00 2001 From: mitchell Date: Fri, 27 Jan 2023 15:34:02 -0500 Subject: [PATCH 5/9] Move shell detection into its own function. This allows the current shell name/path to be found without creating a new subshell. --- cmd/state/main.go | 3 +- internal/subshell/subshell.go | 62 +++++++++++++++++------------------ 2 files changed, 33 insertions(+), 32 deletions(-) diff --git a/cmd/state/main.go b/cmd/state/main.go index a268a5c4e7..1f241c34de 100644 --- a/cmd/state/main.go +++ b/cmd/state/main.go @@ -82,7 +82,8 @@ func main() { // Set up our output formatter/writer outFlags := parseOutputFlags(os.Args) - out, err := initOutput(outFlags, "", subshell.New(cfg).Shell()) + shellName, _ := subshell.DetectShell(cfg) + out, err := initOutput(outFlags, "", shellName) if err != nil { multilog.Critical("Could not initialize outputer: %s", errs.JoinMessage(err)) os.Stderr.WriteString(locale.Tr("err_main_outputer", err.Error())) diff --git a/internal/subshell/subshell.go b/internal/subshell/subshell.go index 00d51f58c0..2bcc2f7fe0 100644 --- a/internal/subshell/subshell.go +++ b/internal/subshell/subshell.go @@ -88,17 +88,7 @@ type SubShell interface { // New returns the subshell relevant to the current process, but does not activate it func New(cfg sscommon.Configurable) SubShell { - binary := resolveBinaryPath(DetectShellBinary(cfg)) - - name := filepath.Base(binary) - name = strings.TrimSuffix(name, filepath.Ext(name)) - logging.Debug("Detected SHELL: %s", name) - - if runtime.GOOS == "windows" { - // For some reason Go or MSYS doesn't translate paths with spaces correctly, so we have to strip out the - // invalid escape characters for spaces - binary = strings.ReplaceAll(binary, `\ `, ` `) - } + name, path := DetectShell(cfg) var subs SubShell switch name { @@ -117,20 +107,20 @@ func New(cfg sscommon.Configurable) SubShell { rollbar.Error("Unsupported shell: %s", name) // we just want to know what this person is using switch runtime.GOOS { case "windows": - binary = "cmd.exe" + path = resolveBinaryPath("cmd.exe") subs = &cmd.SubShell{} case "darwin": - binary = "zsh" + path = resolveBinaryPath("zsh") subs = &zsh.SubShell{} default: - binary = "bash" + path = resolveBinaryPath("bash") subs = &bash.SubShell{} } - binary = resolveBinaryPath(binary) + } - logging.Debug("Using binary: %s", binary) - subs.SetBinary(binary) + logging.Debug("Using binary: %s", path) + subs.SetBinary(path) env := funk.FilterString(os.Environ(), func(s string) bool { return !strings.HasPrefix(s, constants.ProjectEnvVarName) @@ -182,8 +172,10 @@ func ConfigureAvailableShells(shell SubShell, cfg sscommon.Configurable, env map return nil } -func DetectShellBinary(cfg sscommon.Configurable) (binary string) { +// DetectShell detects the shell relevant to the current process and returns its name and path. +func DetectShell(cfg sscommon.Configurable) (string, string) { configured := cfg.GetString(ConfigKeyShell) + var binary string defer func() { // do not re-write shell binary to config, if the value did not change. if configured == binary { @@ -196,25 +188,33 @@ func DetectShellBinary(cfg sscommon.Configurable) (binary string) { } }() - if binary := os.Getenv("SHELL"); binary != "" { - return binary - } - - if runtime.GOOS == "windows" { + binary = os.Getenv("SHELL") + if binary == "" && runtime.GOOS == "windows" { binary = os.Getenv("ComSpec") - if binary != "" { - return binary - } } - fallback := configured - if fallback == "" { + if binary == "" { + binary = configured + } + if binary == "" { if runtime.GOOS == "windows" { - fallback = "cmd.exe" + binary = "cmd.exe" } else { - fallback = "bash" + binary = "bash" } } - return fallback + path := resolveBinaryPath(binary) + + name := filepath.Base(path) + name = strings.TrimSuffix(name, filepath.Ext(name)) + logging.Debug("Detected SHELL: %s", name) + + if runtime.GOOS == "windows" { + // For some reason Go or MSYS doesn't translate paths with spaces correctly, so we have to strip out the + // invalid escape characters for spaces + path = strings.ReplaceAll(path, `\ `, ` `) + } + + return name, path } From a8d6d94a52ed28e33ce0fdb6c993d941d2b1a359 Mon Sep 17 00:00:00 2001 From: mitchell Date: Fri, 27 Jan 2023 15:42:47 -0500 Subject: [PATCH 6/9] Added comments about error handling. --- internal/output/progress_unix.go | 3 ++- internal/output/progress_win.go | 16 +++++++++------- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/internal/output/progress_unix.go b/internal/output/progress_unix.go index e1cdd640f0..bbb253a673 100644 --- a/internal/output/progress_unix.go +++ b/internal/output/progress_unix.go @@ -4,5 +4,6 @@ package output func (d *Spinner) moveCaretBackInCommandPrompt(n int) { - // No-op + // No-op (logging to Rollbar every tick would be a disaster) + // Rely on manual and unit testing to catch any errors in display. } diff --git a/internal/output/progress_win.go b/internal/output/progress_win.go index 8416624c95..af6117301f 100644 --- a/internal/output/progress_win.go +++ b/internal/output/progress_win.go @@ -40,11 +40,13 @@ func (d *Spinner) moveCaretBackInCommandPrompt(n int) { handle := syscall.Handle(os.Stdout.Fd()) var csbi consoleScreenBufferInfo - _, _, _ = procGetConsoleScreenBufferInfo.Call(uintptr(handle), uintptr(unsafe.Pointer(&csbi))) - - var cursor coord - cursor.x = csbi.cursorPosition.x + short(-n) - cursor.y = csbi.cursorPosition.y - - _, _, _ = procSetConsoleCursorPosition.Call(uintptr(handle), uintptr(*(*int32)(unsafe.Pointer(&cursor)))) + if _, _, err := procGetConsoleScreenBufferInfo.Call(uintptr(handle), uintptr(unsafe.Pointer(&csbi))); err != nil { + var cursor coord + cursor.x = csbi.cursorPosition.x + short(-n) + cursor.y = csbi.cursorPosition.y + + _, _, _ = procSetConsoleCursorPosition.Call(uintptr(handle), uintptr(*(*int32)(unsafe.Pointer(&cursor)))) + } + // Note: do not log or report errors because they would be logged/reported for every tick, which + // could be disastrous. Instead, rely on manual and unit testing to catch any errors in display. } From 920975dcc49a13f5da80c32044b2534a8bc76ed6 Mon Sep 17 00:00:00 2001 From: mitchell Date: Fri, 27 Jan 2023 17:02:05 -0500 Subject: [PATCH 7/9] Handle unsupported shells in `subshell.DetectShell()`. --- internal/subshell/subshell.go | 40 +++++++++++++++++++++++++---------- 1 file changed, 29 insertions(+), 11 deletions(-) diff --git a/internal/subshell/subshell.go b/internal/subshell/subshell.go index 2bcc2f7fe0..d645109966 100644 --- a/internal/subshell/subshell.go +++ b/internal/subshell/subshell.go @@ -92,31 +92,26 @@ func New(cfg sscommon.Configurable) SubShell { var subs SubShell switch name { - case "bash": + case bash.Name: subs = &bash.SubShell{} - case "zsh": + case zsh.Name: subs = &zsh.SubShell{} - case "tcsh": + case tcsh.Name: subs = &tcsh.SubShell{} - case "fish": + case fish.Name: subs = &fish.SubShell{} - case "cmd": + case cmd.Name: subs = &cmd.SubShell{} default: - logging.Debug("Unsupported shell: %s, defaulting to OS default.", name) - rollbar.Error("Unsupported shell: %s", name) // we just want to know what this person is using + rollbar.Error("subshell.DetectShell did not return a known name: %s", name) switch runtime.GOOS { case "windows": - path = resolveBinaryPath("cmd.exe") subs = &cmd.SubShell{} case "darwin": - path = resolveBinaryPath("zsh") subs = &zsh.SubShell{} default: - path = resolveBinaryPath("bash") subs = &bash.SubShell{} } - } logging.Debug("Using binary: %s", path) @@ -216,5 +211,28 @@ func DetectShell(cfg sscommon.Configurable) (string, string) { path = strings.ReplaceAll(path, `\ `, ` `) } + isKnownShell := false + for _, ssName := range []string{bash.Name, cmd.Name, fish.Name, tcsh.Name, zsh.Name} { + if name == ssName { + isKnownShell = true + break + } + } + if !isKnownShell { + logging.Debug("Unsupported shell: %s, defaulting to OS default.", name) + rollbar.Error("Unsupported shell: %s", name) // we just want to know what this person is using + switch runtime.GOOS { + case "windows": + name = cmd.Name + path = resolveBinaryPath("cmd.exe") + case "darwin": + name = zsh.Name + path = resolveBinaryPath("zsh") + default: + name = bash.Name + path = resolveBinaryPath("bash") + } + } + return name, path } From 2ac1fc082a88e624f334fe41219bd8871a9f4ddf Mon Sep 17 00:00:00 2001 From: mitchell Date: Fri, 27 Jan 2023 17:11:02 -0500 Subject: [PATCH 8/9] Report first instance of a spinner error to rollbar. --- internal/output/progress.go | 13 +++++++------ internal/output/progress_unix.go | 10 ++++++++-- internal/output/progress_win.go | 13 ++++++++++--- 3 files changed, 25 insertions(+), 11 deletions(-) diff --git a/internal/output/progress.go b/internal/output/progress.go index 53f2565a0c..f1629f17e2 100644 --- a/internal/output/progress.go +++ b/internal/output/progress.go @@ -9,11 +9,12 @@ import ( const moveCaretBackEscapeSequence = "\x1b[%dD" // %d is the number of characters to move back type Spinner struct { - frame int - frames []string - out Outputer - stop chan struct{} - interval time.Duration + frame int + frames []string + out Outputer + stop chan struct{} + interval time.Duration + reportedError bool } var _ Marshaller = &Spinner{} @@ -27,7 +28,7 @@ func StartSpinner(out Outputer, msg string, interval time.Duration) *Spinner { if out.Config().Interactive { frames = []string{`|`, `/`, `-`, `\`} } - d := &Spinner{0, frames, out, make(chan struct{}, 1), interval} + d := &Spinner{0, frames, out, make(chan struct{}, 1), interval, false} if msg != "" { d.out.Fprint(d.out.Config().ErrWriter, strings.TrimSuffix(msg, " ")+" ") diff --git a/internal/output/progress_unix.go b/internal/output/progress_unix.go index bbb253a673..f6b96ead9e 100644 --- a/internal/output/progress_unix.go +++ b/internal/output/progress_unix.go @@ -3,7 +3,13 @@ package output +import ( + "github.com/ActiveState/cli/internal/rollbar" +) + func (d *Spinner) moveCaretBackInCommandPrompt(n int) { - // No-op (logging to Rollbar every tick would be a disaster) - // Rely on manual and unit testing to catch any errors in display. + if !d.reportedError { + rollbar.Error("Incorrectly detected Windows command prompt in Unix environment") + d.reportedError = true + } } diff --git a/internal/output/progress_win.go b/internal/output/progress_win.go index af6117301f..70b3d948c7 100644 --- a/internal/output/progress_win.go +++ b/internal/output/progress_win.go @@ -7,6 +7,8 @@ import ( "os" "syscall" "unsafe" + + "github.com/ActiveState/cli/internal/rollbar" ) var kernel32 = syscall.NewLazyDLL("kernel32.dll") @@ -45,8 +47,13 @@ func (d *Spinner) moveCaretBackInCommandPrompt(n int) { cursor.x = csbi.cursorPosition.x + short(-n) cursor.y = csbi.cursorPosition.y - _, _, _ = procSetConsoleCursorPosition.Call(uintptr(handle), uintptr(*(*int32)(unsafe.Pointer(&cursor)))) + _, _, err2 = procSetConsoleCursorPosition.Call(uintptr(handle), uintptr(*(*int32)(unsafe.Pointer(&cursor)))) + if err2 != nil && !d.reportedError { + rollbar.Error("Error calling SetConsoleCursorPosition: %v", err2) + d.reportedError = true + } + } else if !d.reportedError { + rollbar.Error("Error calling GetConsoleScreenBufferInfo: %v", err) + d.reportedError = true } - // Note: do not log or report errors because they would be logged/reported for every tick, which - // could be disastrous. Instead, rely on manual and unit testing to catch any errors in display. } From cde3aad38bcd4b24a75b82aed544ed620854edd9 Mon Sep 17 00:00:00 2001 From: mitchell Date: Fri, 27 Jan 2023 17:13:43 -0500 Subject: [PATCH 9/9] Fixed windows build error. --- internal/output/progress_win.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/output/progress_win.go b/internal/output/progress_win.go index 70b3d948c7..335f6cfe0c 100644 --- a/internal/output/progress_win.go +++ b/internal/output/progress_win.go @@ -47,7 +47,7 @@ func (d *Spinner) moveCaretBackInCommandPrompt(n int) { cursor.x = csbi.cursorPosition.x + short(-n) cursor.y = csbi.cursorPosition.y - _, _, err2 = procSetConsoleCursorPosition.Call(uintptr(handle), uintptr(*(*int32)(unsafe.Pointer(&cursor)))) + _, _, err2 := procSetConsoleCursorPosition.Call(uintptr(handle), uintptr(*(*int32)(unsafe.Pointer(&cursor)))) if err2 != nil && !d.reportedError { rollbar.Error("Error calling SetConsoleCursorPosition: %v", err2) d.reportedError = true