Disable terminal echo while State Tool is running.

The exception is for prompts and subshells (which temporarily re-enable echoing).
ActiveState · Nov 13, 2023 · 5dbd4bf · 5dbd4bf
1 parent e693344
commit 5dbd4bf
Show file tree

Hide file tree

Showing 15 changed files with 84 additions and 27 deletions.
diff --git a/cmd/state/main.go b/cmd/state/main.go
@@ -207,6 +207,13 @@ func run(args []string, isInteractive bool, cfg *config.Instance, out output.Out
 
 	// Set up conditional, which accesses a lot of primer data
 	sshell := subshell.New(cfg)
+	if isInteractive {
+		// Disable terminal echo while State Tool is running.
+		// Other than in prompts and subshells (which temporarily re-enable echo), user typing should
+		// not interfere with output (e.g. runtime progress bars).
+		sshell.TurnOffEcho()
+		defer sshell.TurnOnEcho()
+	}
 
 	conditional := constraints.NewPrimeConditional(auth, pj, sshell.Shell())
 	project.RegisterConditional(conditional)

diff --git a/internal/osutils/termecho/termecho.go b/internal/osutils/termecho/termecho.go
diff --git a/internal/runbits/activation/activation.go b/internal/runbits/activation/activation.go
@@ -67,6 +67,8 @@ func ActivateAndWait(
 	if err := ss.Activate(proj, cfg, out); err != nil {
 		return locale.WrapError(err, "error_could_not_activate_subshell", "Could not activate a new subshell.")
 	}
+	ss.TurnOnEcho() // temporarily re-enable echo while the subshell is active
+	defer ss.TurnOffEcho()
 
 	a, err := process.NewActivation(cfg, os.Getpid())
 	if err != nil {

diff --git a/internal/runbits/runtime/progress/progress.go b/internal/runbits/runtime/progress/progress.go
@@ -7,17 +7,17 @@ import (
 	"sync"
 	"time"
 
+	"github.com/go-openapi/strfmt"
+	"github.com/vbauerster/mpb/v7"
+	"golang.org/x/net/context"
+
 	"github.com/ActiveState/cli/internal/errs"
 	"github.com/ActiveState/cli/internal/locale"
 	"github.com/ActiveState/cli/internal/logging"
 	"github.com/ActiveState/cli/internal/multilog"
-	"github.com/ActiveState/cli/internal/osutils/termecho"
 	"github.com/ActiveState/cli/internal/output"
 	"github.com/ActiveState/cli/pkg/platform/runtime/artifact"
 	"github.com/ActiveState/cli/pkg/platform/runtime/setup/events"
-	"github.com/go-openapi/strfmt"
-	"github.com/vbauerster/mpb/v7"
-	"golang.org/x/net/context"
 )
 
 type step struct {
@@ -89,10 +89,6 @@ type ProgressDigester struct {
 }
 
 func NewProgressIndicator(w io.Writer, out output.Outputer) *ProgressDigester {
-	err := termecho.Off()
-	if err != nil {
-		multilog.Error("Unable to turn off terminal echoing: %v", errs.JoinMessage(err))
-	}
 	ctx, cancel := context.WithCancel(context.Background())
 	return &ProgressDigester{
 		mainProgress: mpb.NewWithContext(
@@ -384,10 +380,5 @@ Still expecting:
 	// Blank line to separate progress from rest of output
 	p.out.Notice("")
 
-	err := termecho.On()
-	if err != nil {
-		multilog.Error("Unable to turn terminal echoing back on: %v", errs.JoinMessage(err))
-	}
-
 	return nil
 }
diff --git a/internal/subshell/bash/bash.go b/internal/subshell/bash/bash.go
@@ -15,6 +15,7 @@ import (
 	"github.com/ActiveState/cli/internal/osutils/user"
 	"github.com/ActiveState/cli/internal/output"
 	"github.com/ActiveState/cli/internal/subshell/sscommon"
+	"github.com/ActiveState/cli/internal/subshell/termecho"
 	"github.com/ActiveState/cli/pkg/project"
 )
 
@@ -219,3 +220,11 @@ func (v *SubShell) IsAvailable() bool {
 	}
 	return fileutils.FileExists(rcFile)
 }
+
+func (v *SubShell) TurnOffEcho() {
+	termecho.Off()
+}
+
+func (v *SubShell) TurnOnEcho() {
+	termecho.Off()
+}
diff --git a/internal/subshell/cmd/cmd.go b/internal/subshell/cmd/cmd.go
@@ -12,6 +12,7 @@ import (
 	"github.com/ActiveState/cli/internal/osutils"
 	"github.com/ActiveState/cli/internal/output"
 	"github.com/ActiveState/cli/internal/subshell/sscommon"
+	"github.com/ActiveState/cli/internal/subshell/termecho"
 	"github.com/ActiveState/cli/pkg/project"
 )
 
@@ -203,3 +204,11 @@ func (v *SubShell) IsActive() bool {
 func (v *SubShell) IsAvailable() bool {
 	return runtime.GOOS == "windows"
 }
+
+func (v *SubShell) TurnOffEcho() {
+	termecho.Off()
+}
+
+func (v *SubShell) TurnOnEcho() {
+	termecho.Off()
+}
diff --git a/internal/subshell/fish/fish.go b/internal/subshell/fish/fish.go
@@ -14,6 +14,7 @@ import (
 	"github.com/ActiveState/cli/internal/osutils/user"
 	"github.com/ActiveState/cli/internal/output"
 	"github.com/ActiveState/cli/internal/subshell/sscommon"
+	"github.com/ActiveState/cli/internal/subshell/termecho"
 	"github.com/ActiveState/cli/pkg/project"
 )
 
@@ -195,3 +196,11 @@ func (v *SubShell) IsAvailable() bool {
 	}
 	return fileutils.FileExists(rcFile)
 }
+
+func (v *SubShell) TurnOffEcho() {
+	termecho.Off()
+}
+
+func (v *SubShell) TurnOnEcho() {
+	termecho.Off()
+}
diff --git a/internal/subshell/subshell.go b/internal/subshell/subshell.go
@@ -84,6 +84,12 @@ type SubShell interface {
 
 	// IsAvailable returns whether the shell is available on the system
 	IsAvailable() bool
+
+	// TurnOffEcho turns off input echoing.
+	TurnOffEcho()
+
+	// TurnOnEcho turns on input echoing.
+	TurnOnEcho()
 }
 
 // New returns the subshell relevant to the current process, but does not activate it

diff --git a/internal/subshell/tcsh/tcsh.go b/internal/subshell/tcsh/tcsh.go
@@ -13,6 +13,7 @@ import (
 	"github.com/ActiveState/cli/internal/osutils/user"
 	"github.com/ActiveState/cli/internal/output"
 	"github.com/ActiveState/cli/internal/subshell/sscommon"
+	"github.com/ActiveState/cli/internal/subshell/termecho"
 	"github.com/ActiveState/cli/pkg/project"
 )
 
@@ -189,3 +190,11 @@ func (v *SubShell) IsAvailable() bool {
 	}
 	return fileutils.FileExists(rcFile)
 }
+
+func (v *SubShell) TurnOffEcho() {
+	termecho.Off()
+}
+
+func (v *SubShell) TurnOnEcho() {
+	termecho.Off()
+}
diff --git a/internal/subshell/termecho/termecho.go b/internal/subshell/termecho/termecho.go
@@ -0,0 +1,20 @@
+package termecho
+
+import (
+	"github.com/ActiveState/cli/internal/errs"
+	"github.com/ActiveState/cli/internal/multilog"
+)
+
+func Off() {
+	err := toggle(false)
+	if err != nil {
+		multilog.Error("Unable to turn off terminal echoing: %v", errs.JoinMessage(err))
+	}
+}
+
+func On() {
+	err := toggle(true)
+	if err != nil {
+		multilog.Error("Unable to turn off terminal echoing: %v", errs.JoinMessage(err))
+	}
+}
diff --git a/internal/osutils/termecho/termecho_darwin.go → ...rnal/subshell/termecho/termecho_darwin.go b/internal/osutils/termecho/termecho_darwin.go → ...rnal/subshell/termecho/termecho_darwin.go
diff --git a/internal/osutils/termecho/termecho_linux.go → internal/subshell/termecho/termecho_linux.go b/internal/osutils/termecho/termecho_linux.go → internal/subshell/termecho/termecho_linux.go
diff --git a/internal/osutils/termecho/termecho_unix.go → internal/subshell/termecho/termecho_unix.go b/internal/osutils/termecho/termecho_unix.go → internal/subshell/termecho/termecho_unix.go
diff --git a/...rnal/osutils/termecho/termecho_windows.go → ...nal/subshell/termecho/termecho_windows.go b/...rnal/osutils/termecho/termecho_windows.go → ...nal/subshell/termecho/termecho_windows.go
@@ -4,7 +4,6 @@ import (
 	"os"
 
 	"github.com/ActiveState/cli/internal/errs"
-	"github.com/ActiveState/cli/internal/logging"
 	"golang.org/x/sys/windows"
 )
 
@@ -13,10 +12,6 @@ func toggle(on bool) error {
 	var mode uint32
 	err := windows.GetConsoleMode(fd, &mode)
 	if err != nil {
-		if shell := os.Getenv("SHELL"); shell != "" {
-			logging.Debug("Cannot turn off terminal echo in %s", shell)
-			return nil
-		}
 		return errs.Wrap(err, "Error calling GetConsoleMode")
 	}
 

diff --git a/internal/subshell/zsh/zsh.go b/internal/subshell/zsh/zsh.go
@@ -18,6 +18,7 @@ import (
 	"github.com/ActiveState/cli/internal/osutils/user"
 	"github.com/ActiveState/cli/internal/output"
 	"github.com/ActiveState/cli/internal/subshell/sscommon"
+	"github.com/ActiveState/cli/internal/subshell/termecho"
 	"github.com/ActiveState/cli/pkg/project"
 )
 
@@ -239,3 +240,11 @@ func (v *SubShell) IsAvailable() bool {
 	}
 	return fileutils.FileExists(rcFile)
 }
+
+func (v *SubShell) TurnOffEcho() {
+	termecho.Off()
+}
+
+func (v *SubShell) TurnOnEcho() {
+	termecho.Off()
+}