diff --git a/docs/docs/changelog.md b/docs/docs/changelog.md index 562383f2..ba1ffc9a 100644 --- a/docs/docs/changelog.md +++ b/docs/docs/changelog.md @@ -5,6 +5,7 @@ title: Changelog ## [Unreleased](https://github.com/lets-cli/lets/releases/tag/v0.0.X) +* `[Fixed]` `source <(lets completion -s zsh)` no longer hangs: skip terminal background-color detection when lets runs outside the terminal's foreground process group (e.g. inside process substitution), where the query would suspend the process with SIGTTIN/SIGTTOU. * `[Fixed]` Add spacing between the `EXAMPLES` help title and its examples. * `[Fixed]` Align styled error output to the left while preserving the padded error badge. * `[Docs]` Convert design specs into ADRs. diff --git a/go.mod b/go.mod index 4aa5c593..1e552c65 100644 --- a/go.mod +++ b/go.mod @@ -69,7 +69,7 @@ require ( github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 github.com/lithammer/dedent v1.1.0 github.com/spf13/pflag v1.0.9 - golang.org/x/sys v0.42.0 // indirect + golang.org/x/sys v0.42.0 gopkg.in/check.v1 v1.0.0-20200902074654-038fdea0a05b // indirect gopkg.in/yaml.v3 v3.0.1 ) diff --git a/internal/config/config/config.go b/internal/config/config/config.go index 6d893d1b..90d0509b 100644 --- a/internal/config/config/config.go +++ b/internal/config/config/config.go @@ -259,7 +259,7 @@ func (c *Config) readMixin(mixin *Mixin) error { // ensureRemoteConfig's behavior for the top-level remote config. cachedData, readErr := rm.tryRead() if readErr != nil { - return fmt.Errorf("download failed (%w); also failed to read cached version: %v", downloadErr, readErr) + return fmt.Errorf("download failed (%w); also failed to read cached version: %w", downloadErr, readErr) } if cachedData != nil { diff --git a/internal/logging/formatter.go b/internal/logging/formatter.go index 20508b7d..fc995437 100644 --- a/internal/logging/formatter.go +++ b/internal/logging/formatter.go @@ -4,7 +4,6 @@ import ( "bytes" "fmt" "io" - "os" "strings" "unicode" @@ -12,6 +11,7 @@ import ( "github.com/charmbracelet/x/term" "github.com/fatih/color" "github.com/lets-cli/fang" + "github.com/lets-cli/lets/internal/theme" log "github.com/sirupsen/logrus" ) @@ -44,8 +44,7 @@ func newFormatter(errWriter io.Writer, cs fang.ColorSchemeFunc) *Formatter { return f } - isDark := lipgloss.HasDarkBackground(os.Stdin, file) - scheme := cs(lipgloss.LightDark(isDark)) + scheme := cs(lipgloss.LightDark(theme.IsDarkBackground(file))) w, _, err := term.GetSize(file.Fd()) if err != nil || w == 0 { diff --git a/internal/theme/theme.go b/internal/theme/theme.go index 599c9381..6cec9abd 100644 --- a/internal/theme/theme.go +++ b/internal/theme/theme.go @@ -8,6 +8,7 @@ import ( "github.com/charmbracelet/x/exp/charmtone" "github.com/charmbracelet/x/term" "github.com/lets-cli/fang" + "github.com/lets-cli/lets/internal/util" ) // Supported theme names. @@ -39,10 +40,22 @@ func ColorSchemeByName(name string) fang.ColorSchemeFunc { } } +// IsDarkBackground reports whether the terminal behind out has a dark background. +// Detection queries the terminal (writes OSC 11 to out, reads the reply from stdin), +// which suspends a background process group with SIGTTIN/SIGTTOU — so when the +// process is not in the foreground, skip the query and assume dark (lipgloss's +// own fallback). +func IsDarkBackground(out term.File) bool { + if !util.IsForegroundProcess(out.Fd()) { + return true + } + + return lipgloss.HasDarkBackground(os.Stdin, out) +} + // ProgressColorsByName resolves a theme name to fill and empty colors for download progress. func ProgressColorsByName(name string, out term.File) (color.Color, color.Color) { - isDark := lipgloss.HasDarkBackground(os.Stdin, out) - scheme := ColorSchemeByName(name)(lipgloss.LightDark(isDark)) + scheme := ColorSchemeByName(name)(lipgloss.LightDark(IsDarkBackground(out))) return ProgressColors(scheme) } diff --git a/internal/util/foreground.go b/internal/util/foreground.go new file mode 100644 index 00000000..a8600f6b --- /dev/null +++ b/internal/util/foreground.go @@ -0,0 +1,19 @@ +package util + +import ( + "golang.org/x/sys/unix" +) + +// IsForegroundProcess reports whether this process belongs to the foreground +// process group of the terminal referred to by fd. Reading or reconfiguring a +// terminal from a background process group suspends the process with +// SIGTTIN/SIGTTOU — e.g. inside process substitution: +// `source <(lets completion -s zsh)`. +func IsForegroundProcess(fd uintptr) bool { + pgrp, err := unix.IoctlGetInt(int(fd), unix.TIOCGPGRP) + if err != nil { + return false + } + + return pgrp == unix.Getpgrp() +} diff --git a/tests/completion.bats b/tests/completion.bats index df5dfbf4..200c968c 100644 --- a/tests/completion.bats +++ b/tests/completion.bats @@ -51,3 +51,13 @@ setup() { assert_line --index 0 "--debug[Run with debug]" assert_line --index 1 "--env[Set env]" } + +# Regression: querying the terminal background color from a background process +# group suspends the process with SIGTTOU, so `source <(lets completion -s zsh)` +# in an interactive (job-control) shell hung forever. `script` allocates a PTY, +# `set -m` enables job control like an interactive shell. +@test "completion: source <(lets completion -s zsh) must not hang under job control" { + run timeout 10 script -qec "zsh -c 'set -m; source <(lets completion -s zsh); echo SOURCED_OK'" /dev/null + assert_success + assert_output --partial "SOURCED_OK" +}