fix(weed/shell): suppress prompt when piped (#8990)

* fix(weed/shell): suppress prompt when stdin or stdout is not a TTY

When piping weed shell output (e.g. `echo "s3.user.list" | weed shell | jq`),
the "> " prompt was written to stdout, breaking JSON parsers.

`liner.TerminalSupported()` only checks platform support, not whether
stdin/stdout are actual TTYs. Add explicit checks using `term.IsTerminal()`
so the shell falls back to the non-interactive scanner path when piped.

Fixes #8962

* fix(weed/shell): suppress informational logs unless -verbose is set

Suppress glog info messages and connection status logs on stderr by
default. Add -verbose flag to opt in to the previous noisy behavior.
This keeps piped output clean (e.g. `echo "s3.user.list" | weed shell | jq`).

* fix(weed/shell): defer liner init until after TTY check

Move liner.NewLiner() and related setup (history, completion, interrupt
handler) inside the interactive block so the terminal is not put into
raw mode when stdout is redirected. Previously, liner would set raw mode
unconditionally at startup, leaving the terminal broken when falling
back to the scanner path.

Addresses review feedback from gemini-code-assist.

* refactor(weed/shell): consolidate verbose logging into single block

Group all verbose stderr output within one conditional block instead of
scattering three separate if-verbose checks around the filer logic.

Addresses review feedback from gemini-code-assist.

* fix(weed/shell): clean up global liner state and suppress logtostderr

- Set line=nil after Close() to prevent stale state if RunShell is
  called again (e.g. in tests)
- Add nil check in OnInterrupt handler for non-interactive sessions
- Also set logtostderr=false when not verbose, in case it was enabled

Addresses review feedback from gemini-code-assist.

* refactor(weed/shell): make liner state local to eliminate data race

Replace the package-level `line` variable with a local variable in
RunShell, passing it explicitly to setCompletionHandler, loadHistory,
and saveHistory. This eliminates a data race between the OnInterrupt
goroutine and the defer that previously set the global to nil.

Addresses review feedback from gemini-code-assist.

* rename(weed/shell): rename -verbose flag to -debug

Avoid conflict with -verbose flags already used by individual shell
commands (e.g. ec.encode, volume.fix.replication, volume.check.disk).
This commit is contained in:
Chris Lu
2026-04-08 13:07:15 -07:00
committed by GitHub
parent ab8c982cec
commit 7f3908297c
3 changed files with 42 additions and 22 deletions
+6 -1
View File
@@ -15,6 +15,7 @@ var (
shellOptions shell.ShellOptions
shellInitialFiler *string
shellCluster *string
shellDebug *bool
)
func init() {
@@ -23,6 +24,7 @@ func init() {
shellOptions.FilerGroup = cmdShell.Flag.String("filerGroup", "", "filerGroup for the filers")
shellInitialFiler = cmdShell.Flag.String("filer", "", "filer host and port for initial connection, e.g. localhost:8888")
shellCluster = cmdShell.Flag.String("cluster", "", "cluster defined in shell.toml")
shellDebug = cmdShell.Flag.Bool("debug", false, "print informational logs to stderr")
}
var cmdShell = &Command{
@@ -61,7 +63,10 @@ func runShell(command *Command, args []string) bool {
filerAddress = viper.GetString("cluster." + cluster + ".filer")
}
shellOptions.FilerAddress = pb.ServerAddress(filerAddress)
fmt.Fprintf(os.Stderr, "master: %s filer: %s\n", *shellOptions.Masters, shellOptions.FilerAddress)
shellOptions.Debug = *shellDebug
if shellOptions.Debug {
fmt.Fprintf(os.Stderr, "master: %s filer: %s\n", *shellOptions.Masters, shellOptions.FilerAddress)
}
shell.RunShell(shellOptions)
+1
View File
@@ -28,6 +28,7 @@ type ShellOptions struct {
FilerGroup *string
FilerAddress pb.ServerAddress
Directory string
Debug bool
}
type CommandEnv struct {
+35 -21
View File
@@ -18,30 +18,40 @@ import (
"github.com/seaweedfs/seaweedfs/weed/util/grace"
"github.com/peterh/liner"
flag "github.com/seaweedfs/seaweedfs/weed/util/fla9"
"golang.org/x/term"
)
var (
line *liner.State
historyPath = path.Join(os.TempDir(), "weed-shell")
)
var historyPath = path.Join(os.TempDir(), "weed-shell")
func RunShell(options ShellOptions) {
slices.SortFunc(Commands, func(a, b command) int {
return strings.Compare(a.Name(), b.Name())
})
line = liner.NewLiner()
defer line.Close()
grace.OnInterrupt(func() {
line.Close()
})
line.SetCtrlCAborts(true)
line.SetTabCompletionStyle(liner.TabPrints)
if !options.Debug {
flag.Set("alsologtostderr", "false")
flag.Set("logtostderr", "false")
}
setCompletionHandler()
loadHistory()
interactive := liner.TerminalSupported() && term.IsTerminal(int(os.Stdin.Fd())) && term.IsTerminal(int(os.Stdout.Fd()))
defer saveHistory()
var line *liner.State
if interactive {
line = liner.NewLiner()
defer line.Close()
grace.OnInterrupt(func() {
line.Close()
})
line.SetCtrlCAborts(true)
line.SetTabCompletionStyle(liner.TabPrints)
setCompletionHandler(line)
loadHistory(line)
defer saveHistory(line)
}
commandEnv := NewCommandEnv(&options)
@@ -65,15 +75,19 @@ func RunShell(options ShellOptions) {
}
return nil
})
fmt.Fprintf(os.Stderr, "master: %s ", *options.Masters)
if len(filers) > 0 {
fmt.Fprintf(os.Stderr, "filers: %v", filers)
commandEnv.option.FilerAddress = filers[rand.IntN(len(filers))]
}
fmt.Fprintln(os.Stderr)
if options.Debug {
if len(filers) > 0 {
fmt.Fprintf(os.Stderr, "master: %s filers: %v\n", *options.Masters, filers)
} else {
fmt.Fprintf(os.Stderr, "master: %s\n", *options.Masters)
}
}
}
if liner.TerminalSupported() {
if interactive {
for {
cmd, err := line.Prompt("> ")
if err != nil {
@@ -228,7 +242,7 @@ func printHelp(cmds []string) {
}
}
func setCompletionHandler() {
func setCompletionHandler(line *liner.State) {
line.SetCompleter(func(line string) (c []string) {
for _, i := range Commands {
if strings.HasPrefix(i.Name(), strings.ToLower(line)) {
@@ -239,14 +253,14 @@ func setCompletionHandler() {
})
}
func loadHistory() {
func loadHistory(line *liner.State) {
if f, err := os.Open(historyPath); err == nil {
line.ReadHistory(f)
f.Close()
}
}
func saveHistory() {
func saveHistory(line *liner.State) {
if f, err := os.Create(historyPath); err != nil {
fmt.Fprintf(os.Stderr, "Error creating history file: %v\n", err)
} else {