From 146037733472eb74429d6c053ccbb8087fe70bca Mon Sep 17 00:00:00 2001 From: Steve Moyer Date: Mon, 16 Jan 2023 10:57:51 -0500 Subject: feat: detect os.Stdin/os.Stdout mode --- commands/execenv/env.go | 33 +++++++++++++++++++++--- commands/execenv/env_test.go | 61 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 3 deletions(-) create mode 100644 commands/execenv/env_test.go (limited to 'commands') diff --git a/commands/execenv/env.go b/commands/execenv/env.go index d2d1c301..990bd726 100644 --- a/commands/execenv/env.go +++ b/commands/execenv/env.go @@ -20,19 +20,46 @@ const RootCommandName = "git-bug" const gitBugNamespace = "git-bug" +type IOMode int + +const ( + UnknownIOMode IOMode = iota + TerminalIOMode + PipedOrRedirectedIOMode +) + +func getIOMode(io *os.File) IOMode { + info, err := io.Stat() + if err != nil { + panic("only os.StdIn or os.Stdout should be passed to this method") + } + + if (info.Mode() & os.ModeCharDevice) == os.ModeCharDevice { + return TerminalIOMode + } + + return PipedOrRedirectedIOMode +} + // Env is the environment of a command type Env struct { Repo repository.ClockedRepo Backend *cache.RepoCache + In io.Reader + InMode IOMode Out Out + OutMode IOMode Err Out } func NewEnv() *Env { return &Env{ - Repo: nil, - Out: out{Writer: os.Stdout}, - Err: out{Writer: os.Stderr}, + Repo: nil, + In: os.Stdin, + InMode: getIOMode(os.Stdin), + Out: out{Writer: os.Stdout}, + OutMode: getIOMode(os.Stdout), + Err: out{Writer: os.Stderr}, } } diff --git a/commands/execenv/env_test.go b/commands/execenv/env_test.go new file mode 100644 index 00000000..11ebdca1 --- /dev/null +++ b/commands/execenv/env_test.go @@ -0,0 +1,61 @@ +package execenv + +import ( + "os" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestGetIOMode(t *testing.T) { + r, w, err := os.Pipe() + require.NoError(t, err) + + testcases := []struct { + name string + in *os.File + out *os.File + expInMode IOMode + expOutMode IOMode + }{ + { + name: "neither redirected", + in: os.Stdin, + out: os.Stdout, + expInMode: TerminalIOMode, + expOutMode: TerminalIOMode, + }, + { + name: "in redirected", + in: w, + out: os.Stdout, + expInMode: TerminalIOMode, + expOutMode: TerminalIOMode, + }, + { + name: "out redirected", + in: os.Stdin, + out: r, + expInMode: TerminalIOMode, + expOutMode: TerminalIOMode, + }, + { + name: "both redirected", + in: w, + out: r, + expInMode: PipedOrRedirectedIOMode, + expOutMode: PipedOrRedirectedIOMode, + }, + } + + for i := range testcases { + testcase := testcases[i] + + t.Run(testcase.name, func(t *testing.T) { + t.Parallel() + + env := NewEnv() + require.NotNil(t, env) + }) + } +} -- cgit From a73640150d8f5c8492140f9d81e7b84542b95763 Mon Sep 17 00:00:00 2001 From: Steve Moyer Date: Tue, 17 Jan 2023 06:38:00 -0500 Subject: feat: use isatty to detect a Termios instead --- commands/execenv/env.go | 48 ++++++++++++++--------------------------- commands/execenv/env_test.go | 20 ++++++++--------- commands/execenv/env_testing.go | 6 ++---- 3 files changed, 28 insertions(+), 46 deletions(-) (limited to 'commands') diff --git a/commands/execenv/env.go b/commands/execenv/env.go index 990bd726..01c00a4a 100644 --- a/commands/execenv/env.go +++ b/commands/execenv/env.go @@ -6,6 +6,7 @@ import ( "io" "os" + "github.com/mattn/go-isatty" "github.com/spf13/cobra" "github.com/vbauerster/mpb/v8" "github.com/vbauerster/mpb/v8/decor" @@ -20,46 +21,29 @@ const RootCommandName = "git-bug" const gitBugNamespace = "git-bug" -type IOMode int - -const ( - UnknownIOMode IOMode = iota - TerminalIOMode - PipedOrRedirectedIOMode -) - -func getIOMode(io *os.File) IOMode { - info, err := io.Stat() - if err != nil { - panic("only os.StdIn or os.Stdout should be passed to this method") - } - - if (info.Mode() & os.ModeCharDevice) == os.ModeCharDevice { - return TerminalIOMode - } - - return PipedOrRedirectedIOMode +func getIOMode(io *os.File) bool { + return !isatty.IsTerminal(io.Fd()) && !isatty.IsCygwinTerminal(io.Fd()) } // Env is the environment of a command type Env struct { - Repo repository.ClockedRepo - Backend *cache.RepoCache - In io.Reader - InMode IOMode - Out Out - OutMode IOMode - Err Out + Repo repository.ClockedRepo + Backend *cache.RepoCache + In io.Reader + InRedirection bool + Out Out + OutRedirection bool + Err Out } func NewEnv() *Env { return &Env{ - Repo: nil, - In: os.Stdin, - InMode: getIOMode(os.Stdin), - Out: out{Writer: os.Stdout}, - OutMode: getIOMode(os.Stdout), - Err: out{Writer: os.Stderr}, + Repo: nil, + In: os.Stdin, + InRedirection: getIOMode(os.Stdin), + Out: out{Writer: os.Stdout}, + OutRedirection: getIOMode(os.Stdout), + Err: out{Writer: os.Stderr}, } } diff --git a/commands/execenv/env_test.go b/commands/execenv/env_test.go index 11ebdca1..3a59f187 100644 --- a/commands/execenv/env_test.go +++ b/commands/execenv/env_test.go @@ -15,36 +15,36 @@ func TestGetIOMode(t *testing.T) { name string in *os.File out *os.File - expInMode IOMode - expOutMode IOMode + expInMode bool + expOutMode bool }{ { name: "neither redirected", in: os.Stdin, out: os.Stdout, - expInMode: TerminalIOMode, - expOutMode: TerminalIOMode, + expInMode: false, + expOutMode: false, }, { name: "in redirected", in: w, out: os.Stdout, - expInMode: TerminalIOMode, - expOutMode: TerminalIOMode, + expInMode: true, + expOutMode: false, }, { name: "out redirected", in: os.Stdin, out: r, - expInMode: TerminalIOMode, - expOutMode: TerminalIOMode, + expInMode: false, + expOutMode: true, }, { name: "both redirected", in: w, out: r, - expInMode: PipedOrRedirectedIOMode, - expOutMode: PipedOrRedirectedIOMode, + expInMode: true, + expOutMode: true, }, } diff --git a/commands/execenv/env_testing.go b/commands/execenv/env_testing.go index 6eb9c69d..a88ae263 100644 --- a/commands/execenv/env_testing.go +++ b/commands/execenv/env_testing.go @@ -49,8 +49,6 @@ func NewTestEnv(t *testing.T) *Env { repo := repository.CreateGoGitTestRepo(t, false) - buf := new(bytes.Buffer) - backend, err := cache.NewRepoCacheNoEvents(repo) require.NoError(t, err) @@ -61,7 +59,7 @@ func NewTestEnv(t *testing.T) *Env { return &Env{ Repo: repo, Backend: backend, - Out: &TestOut{buf}, - Err: &TestOut{buf}, + Out: &TestOut{&bytes.Buffer{}}, + Err: &TestOut{&bytes.Buffer{}}, } } -- cgit From f011452a2d7ed26d522896a1dab090d7ede05cf1 Mon Sep 17 00:00:00 2001 From: Michael Muré Date: Tue, 17 Jan 2023 20:02:31 +0100 Subject: execenv: move terminal detection to Out, introduce the compagnion In --- commands/execenv/env.go | 241 +++++++++++----------------------------- commands/execenv/env_test.go | 54 ++------- commands/execenv/env_testing.go | 37 +++++- commands/execenv/loading.go | 169 ++++++++++++++++++++++++++++ 4 files changed, 273 insertions(+), 228 deletions(-) create mode 100644 commands/execenv/loading.go (limited to 'commands') diff --git a/commands/execenv/env.go b/commands/execenv/env.go index 01c00a4a..46de8401 100644 --- a/commands/execenv/env.go +++ b/commands/execenv/env.go @@ -7,53 +7,61 @@ import ( "os" "github.com/mattn/go-isatty" - "github.com/spf13/cobra" - "github.com/vbauerster/mpb/v8" - "github.com/vbauerster/mpb/v8/decor" "github.com/MichaelMure/git-bug/cache" - "github.com/MichaelMure/git-bug/entities/identity" "github.com/MichaelMure/git-bug/repository" - "github.com/MichaelMure/git-bug/util/interrupt" ) const RootCommandName = "git-bug" const gitBugNamespace = "git-bug" -func getIOMode(io *os.File) bool { - return !isatty.IsTerminal(io.Fd()) && !isatty.IsCygwinTerminal(io.Fd()) -} - // Env is the environment of a command type Env struct { - Repo repository.ClockedRepo - Backend *cache.RepoCache - In io.Reader - InRedirection bool - Out Out - OutRedirection bool - Err Out + Repo repository.ClockedRepo + Backend *cache.RepoCache + In In + Out Out + Err Out } func NewEnv() *Env { return &Env{ - Repo: nil, - In: os.Stdin, - InRedirection: getIOMode(os.Stdin), - Out: out{Writer: os.Stdout}, - OutRedirection: getIOMode(os.Stdout), - Err: out{Writer: os.Stderr}, + Repo: nil, + In: in{Reader: os.Stdin}, + Out: out{Writer: os.Stdout}, + Err: out{Writer: os.Stderr}, } } +type In interface { + io.Reader + + // IsTerminal tells if the input is a user terminal (rather than a buffer, + // a pipe ...), which tells if we can use interactive features. + IsTerminal() bool + + // ForceIsTerminal allow to force the returned value of IsTerminal + // This only works in test scenario. + ForceIsTerminal(value bool) +} + type Out interface { io.Writer + Printf(format string, a ...interface{}) Print(a ...interface{}) Println(a ...interface{}) PrintJSON(v interface{}) error + // IsTerminal tells if the output is a user terminal (rather than a buffer, + // a pipe ...), which tells if we can use colors and other interactive features. + IsTerminal() bool + + // Raw return the underlying io.Writer, or itself if not. + // This is useful if something need to access the raw file descriptor. + Raw() io.Writer + // String returns what have been written in the output before, as a string. // This only works in test scenario. String() string @@ -63,10 +71,24 @@ type Out interface { // Reset clear what has been recorded as written in the output before. // This only works in test scenario. Reset() + // ForceIsTerminal allow to force the returned value of IsTerminal + // This only works in test scenario. + ForceIsTerminal(value bool) +} - // Raw return the underlying io.Writer, or itself if not. - // This is useful if something need to access the raw file descriptor. - Raw() io.Writer +type in struct { + io.Reader +} + +func (i in) IsTerminal() bool { + if f, ok := i.Reader.(*os.File); ok { + return isTerminal(f) + } + return false +} + +func (i in) ForceIsTerminal(_ bool) { + panic("only work with a test env") } type out struct { @@ -94,172 +116,33 @@ func (o out) PrintJSON(v interface{}) error { return nil } -func (o out) String() string { - panic("only work with a test env") -} - -func (o out) Bytes() []byte { - panic("only work with a test env") -} - -func (o out) Reset() { - panic("only work with a test env") +func (o out) IsTerminal() bool { + if f, ok := o.Writer.(*os.File); ok { + return isTerminal(f) + } + return false } func (o out) Raw() io.Writer { return o.Writer } -// LoadRepo is a pre-run function that load the repository for use in a command -func LoadRepo(env *Env) func(*cobra.Command, []string) error { - return func(cmd *cobra.Command, args []string) error { - cwd, err := os.Getwd() - if err != nil { - return fmt.Errorf("unable to get the current working directory: %q", err) - } - - // Note: we are not loading clocks here because we assume that LoadRepo is only used - // when we don't manipulate entities, or as a child call of LoadBackend which will - // read all clocks anyway. - env.Repo, err = repository.OpenGoGitRepo(cwd, gitBugNamespace, nil) - if err == repository.ErrNotARepo { - return fmt.Errorf("%s must be run from within a git Repo", RootCommandName) - } - if err != nil { - return err - } - - return nil - } -} - -// LoadRepoEnsureUser is the same as LoadRepo, but also ensure that the user has configured -// an identity. Use this pre-run function when an error after using the configured user won't -// do. -func LoadRepoEnsureUser(env *Env) func(*cobra.Command, []string) error { - return func(cmd *cobra.Command, args []string) error { - err := LoadRepo(env)(cmd, args) - if err != nil { - return err - } - - _, err = identity.GetUserIdentity(env.Repo) - if err != nil { - return err - } - - return nil - } +func (o out) String() string { + panic("only work with a test env") } -// LoadBackend is a pre-run function that load the repository and the Backend for use in a command -// When using this function you also need to use CloseBackend as a post-run -func LoadBackend(env *Env) func(*cobra.Command, []string) error { - return func(cmd *cobra.Command, args []string) error { - err := LoadRepo(env)(cmd, args) - if err != nil { - return err - } - - var events chan cache.BuildEvent - env.Backend, events = cache.NewRepoCache(env.Repo) - - err = CacheBuildProgressBar(env, events) - if err != nil { - return err - } - - cleaner := func(env *Env) interrupt.CleanerFunc { - return func() error { - if env.Backend != nil { - err := env.Backend.Close() - env.Backend = nil - return err - } - return nil - } - } - - // Cleanup properly on interrupt - interrupt.RegisterCleaner(cleaner(env)) - return nil - } +func (o out) Bytes() []byte { + panic("only work with a test env") } -// LoadBackendEnsureUser is the same as LoadBackend, but also ensure that the user has configured -// an identity. Use this pre-run function when an error after using the configured user won't -// do. -func LoadBackendEnsureUser(env *Env) func(*cobra.Command, []string) error { - return func(cmd *cobra.Command, args []string) error { - err := LoadBackend(env)(cmd, args) - if err != nil { - return err - } - - _, err = identity.GetUserIdentity(env.Repo) - if err != nil { - return err - } - - return nil - } +func (o out) Reset() { + panic("only work with a test env") } -// CloseBackend is a wrapper for a RunE function that will close the Backend properly -// if it has been opened. -// This wrapper style is necessary because a Cobra PostE function does not run if RunE return an error. -func CloseBackend(env *Env, runE func(cmd *cobra.Command, args []string) error) func(*cobra.Command, []string) error { - return func(cmd *cobra.Command, args []string) error { - errRun := runE(cmd, args) - - if env.Backend == nil { - return nil - } - err := env.Backend.Close() - env.Backend = nil - - // prioritize the RunE error - if errRun != nil { - return errRun - } - return err - } +func (o out) ForceIsTerminal(_ bool) { + panic("only work with a test env") } -func CacheBuildProgressBar(env *Env, events chan cache.BuildEvent) error { - var progress *mpb.Progress - var bars = make(map[string]*mpb.Bar) - - for event := range events { - if event.Err != nil { - return event.Err - } - - if progress == nil { - progress = mpb.New(mpb.WithOutput(env.Err.Raw())) - } - - switch event.Event { - case cache.BuildEventCacheIsBuilt: - env.Err.Println("Building cache... ") - case cache.BuildEventStarted: - bars[event.Typename] = progress.AddBar(-1, - mpb.BarRemoveOnComplete(), - mpb.PrependDecorators( - decor.Name(event.Typename, decor.WCSyncSpace), - decor.CountersNoUnit("%d / %d", decor.WCSyncSpace), - ), - mpb.AppendDecorators(decor.Percentage(decor.WCSyncSpace)), - ) - case cache.BuildEventProgress: - bars[event.Typename].SetTotal(event.Total, false) - bars[event.Typename].SetCurrent(event.Progress) - } - } - - if progress != nil { - progress.Shutdown() - } - - return nil +func isTerminal(file *os.File) bool { + return isatty.IsTerminal(file.Fd()) || isatty.IsCygwinTerminal(file.Fd()) } diff --git a/commands/execenv/env_test.go b/commands/execenv/env_test.go index 3a59f187..3fc6e581 100644 --- a/commands/execenv/env_test.go +++ b/commands/execenv/env_test.go @@ -7,55 +7,15 @@ import ( "github.com/stretchr/testify/require" ) -func TestGetIOMode(t *testing.T) { +func TestIsTerminal(t *testing.T) { + // easy way to get a reader and a writer r, w, err := os.Pipe() require.NoError(t, err) - testcases := []struct { - name string - in *os.File - out *os.File - expInMode bool - expOutMode bool - }{ - { - name: "neither redirected", - in: os.Stdin, - out: os.Stdout, - expInMode: false, - expOutMode: false, - }, - { - name: "in redirected", - in: w, - out: os.Stdout, - expInMode: true, - expOutMode: false, - }, - { - name: "out redirected", - in: os.Stdin, - out: r, - expInMode: false, - expOutMode: true, - }, - { - name: "both redirected", - in: w, - out: r, - expInMode: true, - expOutMode: true, - }, - } + require.False(t, isTerminal(r)) + require.False(t, isTerminal(w)) - for i := range testcases { - testcase := testcases[i] - - t.Run(testcase.name, func(t *testing.T) { - t.Parallel() - - env := NewEnv() - require.NotNil(t, env) - }) - } + // golang's testing framework replaces os.Stdin and os.Stdout, so the following doesn't work here + // require.True(t, isTerminal(os.Stdin)) + // require.True(t, isTerminal(os.Stdout)) } diff --git a/commands/execenv/env_testing.go b/commands/execenv/env_testing.go index a88ae263..03fe0430 100644 --- a/commands/execenv/env_testing.go +++ b/commands/execenv/env_testing.go @@ -13,10 +13,26 @@ import ( "github.com/MichaelMure/git-bug/repository" ) +var _ In = &TestIn{} + +type TestIn struct { + *bytes.Buffer + forceIsTerminal bool +} + +func (t *TestIn) ForceIsTerminal(value bool) { + t.forceIsTerminal = value +} + +func (t *TestIn) IsTerminal() bool { + return t.forceIsTerminal +} + var _ Out = &TestOut{} type TestOut struct { *bytes.Buffer + forceIsTerminal bool } func (te *TestOut) Printf(format string, a ...interface{}) { @@ -40,13 +56,29 @@ func (te *TestOut) PrintJSON(v interface{}) error { return nil } +func (te *TestOut) IsTerminal() bool { + return te.forceIsTerminal +} + func (te *TestOut) Raw() io.Writer { return te.Buffer } +func (te *TestOut) ForceIsTerminal(value bool) { + te.forceIsTerminal = value +} + func NewTestEnv(t *testing.T) *Env { t.Helper() + return newTestEnv(t, false) +} + +func NewTestEnvTerminal(t *testing.T) *Env { + t.Helper() + return newTestEnv(t, true) +} +func newTestEnv(t *testing.T, isTerminal bool) *Env { repo := repository.CreateGoGitTestRepo(t, false) backend, err := cache.NewRepoCacheNoEvents(repo) @@ -59,7 +91,8 @@ func NewTestEnv(t *testing.T) *Env { return &Env{ Repo: repo, Backend: backend, - Out: &TestOut{&bytes.Buffer{}}, - Err: &TestOut{&bytes.Buffer{}}, + In: &TestIn{Buffer: &bytes.Buffer{}, forceIsTerminal: isTerminal}, + Out: &TestOut{Buffer: &bytes.Buffer{}, forceIsTerminal: isTerminal}, + Err: &TestOut{Buffer: &bytes.Buffer{}, forceIsTerminal: isTerminal}, } } diff --git a/commands/execenv/loading.go b/commands/execenv/loading.go new file mode 100644 index 00000000..2263f700 --- /dev/null +++ b/commands/execenv/loading.go @@ -0,0 +1,169 @@ +package execenv + +import ( + "fmt" + "os" + + "github.com/spf13/cobra" + "github.com/vbauerster/mpb/v8" + "github.com/vbauerster/mpb/v8/decor" + + "github.com/MichaelMure/git-bug/cache" + "github.com/MichaelMure/git-bug/entities/identity" + "github.com/MichaelMure/git-bug/repository" + "github.com/MichaelMure/git-bug/util/interrupt" +) + +// LoadRepo is a pre-run function that load the repository for use in a command +func LoadRepo(env *Env) func(*cobra.Command, []string) error { + return func(cmd *cobra.Command, args []string) error { + cwd, err := os.Getwd() + if err != nil { + return fmt.Errorf("unable to get the current working directory: %q", err) + } + + // Note: we are not loading clocks here because we assume that LoadRepo is only used + // when we don't manipulate entities, or as a child call of LoadBackend which will + // read all clocks anyway. + env.Repo, err = repository.OpenGoGitRepo(cwd, gitBugNamespace, nil) + if err == repository.ErrNotARepo { + return fmt.Errorf("%s must be run from within a git Repo", RootCommandName) + } + if err != nil { + return err + } + + return nil + } +} + +// LoadRepoEnsureUser is the same as LoadRepo, but also ensure that the user has configured +// an identity. Use this pre-run function when an error after using the configured user won't +// do. +func LoadRepoEnsureUser(env *Env) func(*cobra.Command, []string) error { + return func(cmd *cobra.Command, args []string) error { + err := LoadRepo(env)(cmd, args) + if err != nil { + return err + } + + _, err = identity.GetUserIdentity(env.Repo) + if err != nil { + return err + } + + return nil + } +} + +// LoadBackend is a pre-run function that load the repository and the Backend for use in a command +// When using this function you also need to use CloseBackend as a post-run +func LoadBackend(env *Env) func(*cobra.Command, []string) error { + return func(cmd *cobra.Command, args []string) error { + err := LoadRepo(env)(cmd, args) + if err != nil { + return err + } + + var events chan cache.BuildEvent + env.Backend, events = cache.NewRepoCache(env.Repo) + + err = CacheBuildProgressBar(env, events) + if err != nil { + return err + } + + cleaner := func(env *Env) interrupt.CleanerFunc { + return func() error { + if env.Backend != nil { + err := env.Backend.Close() + env.Backend = nil + return err + } + return nil + } + } + + // Cleanup properly on interrupt + interrupt.RegisterCleaner(cleaner(env)) + return nil + } +} + +// LoadBackendEnsureUser is the same as LoadBackend, but also ensure that the user has configured +// an identity. Use this pre-run function when an error after using the configured user won't +// do. +func LoadBackendEnsureUser(env *Env) func(*cobra.Command, []string) error { + return func(cmd *cobra.Command, args []string) error { + err := LoadBackend(env)(cmd, args) + if err != nil { + return err + } + + _, err = identity.GetUserIdentity(env.Repo) + if err != nil { + return err + } + + return nil + } +} + +// CloseBackend is a wrapper for a RunE function that will close the Backend properly +// if it has been opened. +// This wrapper style is necessary because a Cobra PostE function does not run if RunE return an error. +func CloseBackend(env *Env, runE func(cmd *cobra.Command, args []string) error) func(*cobra.Command, []string) error { + return func(cmd *cobra.Command, args []string) error { + errRun := runE(cmd, args) + + if env.Backend == nil { + return nil + } + err := env.Backend.Close() + env.Backend = nil + + // prioritize the RunE error + if errRun != nil { + return errRun + } + return err + } +} + +func CacheBuildProgressBar(env *Env, events chan cache.BuildEvent) error { + var progress *mpb.Progress + var bars = make(map[string]*mpb.Bar) + + for event := range events { + if event.Err != nil { + return event.Err + } + + if progress == nil { + progress = mpb.New(mpb.WithOutput(env.Err.Raw())) + } + + switch event.Event { + case cache.BuildEventCacheIsBuilt: + env.Err.Println("Building cache... ") + case cache.BuildEventStarted: + bars[event.Typename] = progress.AddBar(-1, + mpb.BarRemoveOnComplete(), + mpb.PrependDecorators( + decor.Name(event.Typename, decor.WCSyncSpace), + decor.CountersNoUnit("%d / %d", decor.WCSyncSpace), + ), + mpb.AppendDecorators(decor.Percentage(decor.WCSyncSpace)), + ) + case cache.BuildEventProgress: + bars[event.Typename].SetTotal(event.Total, false) + bars[event.Typename].SetCurrent(event.Progress) + } + } + + if progress != nil { + progress.Shutdown() + } + + return nil +} -- cgit