refactor: move --no-tty out of option.Common (#1699)

Signed-off-by: Xiaoxuan Wang <xiaoxuanwang@microsoft.com>

Author: wangxiaoxuan273

cmd/oras/internal/option/common.go

@@ -16,55 +16,24 @@ limitations under the License.
 package option
 
 import (
-	"os"
-
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
-	"golang.org/x/term"
 	"oras.land/oras/cmd/oras/internal/output"
 )
 
-const NoTTYFlag = "no-tty"
-
 // Common option struct.
 type Common struct {
 	Printer *output.Printer
-	TTY     *os.File
 	Debug   bool
-
-	noTTY bool
 }
 
 // ApplyFlags applies flags to a command flag set.
 func (opts *Common) ApplyFlags(fs *pflag.FlagSet) {
 	fs.BoolVarP(&opts.Debug, "debug", "d", false, "output debug logs (implies --no-tty)")
-	fs.BoolVarP(&opts.noTTY, NoTTYFlag, "", false, "[Preview] do not show progress output")
 }
 
 // Parse gets target options from user input.
 func (opts *Common) Parse(cmd *cobra.Command) error {
 	opts.Printer = output.NewPrinter(cmd.OutOrStdout(), cmd.OutOrStderr())
-	// use STDERR as TTY output since STDOUT is reserved for pipeable output
-	return opts.parseTTY(os.Stderr)
-}
-
-// parseTTY gets target options from user input.
-func (opts *Common) parseTTY(f *os.File) error {
-	if !opts.noTTY {
-		if opts.Debug {
-			opts.noTTY = true
-		} else if term.IsTerminal(int(f.Fd())) {
-			opts.TTY = f
-		}
-	}
 	return nil
 }
-
-// UpdateTTY updates the TTY value, given the status of --no-tty flag and output
-// path value.
-func (opts *Common) UpdateTTY(flagPresent bool, toSTDOUT bool) {
-	ttyEnforced := flagPresent && !opts.noTTY
-	if opts.noTTY || (toSTDOUT && !ttyEnforced) {
-		opts.TTY = nil
-	}
-}

cmd/oras/internal/option/common_test.go

@@ -0,0 +1,64 @@
+/*
+Copyright The ORAS Authors.
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package option
+
+import (
+	"os"
+
+	"github.com/spf13/cobra"
+	"github.com/spf13/pflag"
+	"golang.org/x/term"
+)
+
+const NoTTYFlag = "no-tty"
+
+// Terminal option struct.
+type Terminal struct {
+	TTY *os.File
+
+	noTTY       bool
+	ttyEnforced bool
+}
+
+// ApplyFlags applies flags to a command flag set.
+func (opts *Terminal) ApplyFlags(fs *pflag.FlagSet) {
+	fs.BoolVarP(&opts.noTTY, NoTTYFlag, "", false, "[Preview] disable progress bars")
+}
+
+// Parse parses the input notty flag.
+func (opts *Terminal) Parse(cmd *cobra.Command) error {
+	opts.ttyEnforced = cmd.Flags().Changed(NoTTYFlag) && !opts.noTTY
+	// use STDERR as TTY output since STDOUT is reserved for pipeable output
+	if !opts.noTTY {
+		f := os.Stderr
+		if term.IsTerminal(int(f.Fd())) {
+			opts.TTY = f
+		}
+	}
+	return nil
+}
+
+// DisableTTY updates the TTY value, given the status of --debug flag, --no-tty flag and output
+// path value.TTY value is set to nil if
+// 1. --no-tty flag is set to true
+// 2. --debug flag is used
+// 3. output path is set to stdout and --no-tty flag is not explicitly set to false
+// (i.e. not --no-tty=false)
+func (opts *Terminal) DisableTTY(debugEnabled, toSTDOUT bool) {
+	if debugEnabled || opts.noTTY || (toSTDOUT && !opts.ttyEnforced) {
+		opts.TTY = nil
+	}
+}

cmd/oras/internal/option/common_unix_test.go

@@ -0,0 +1,90 @@
+//go:build freebsd || linux || netbsd || openbsd || solaris
+
+/*
+Copyright The ORAS Authors.
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package option
+
+import (
+	"os"
+	"reflect"
+	"testing"
+
+	"github.com/spf13/cobra"
+	"oras.land/oras/internal/testutils"
+)
+
+func TestTerminal_Parse(t *testing.T) {
+	_, device, err := testutils.NewPty()
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer device.Close()
+	opts := Terminal{
+		noTTY: false,
+		TTY:   device,
+	}
+	// TTY output
+	if err := opts.Parse(&cobra.Command{}); err != nil {
+		t.Errorf("unexpected error with TTY output: %v", err)
+	}
+}
+
+func TestTerminal_DisableTTY(t *testing.T) {
+	testTTY := &os.File{}
+	tests := []struct {
+		name        string
+		debug       bool
+		toSTDOUT    bool
+		noTTY       bool
+		ttyEnforced bool
+		expectedTTY *os.File
+	}{
+		{
+			"output to STDOUT, --no-tty flag not used, reset TTY", false, false, true, false, nil,
+		},
+		{
+			"output to STDOUT, --no-tty set to true, reset TTY", false, true, true, true, nil,
+		},
+		{
+			"output to STDOUT, --no-tty set to false", false, true, false, true, testTTY,
+		},
+		{
+			"not output to STDOUT, --no-tty flag not used", false, false, false, false, testTTY,
+		},
+		{
+			"not output to STDOUT, --no-tty set to true, reset TTY", false, false, true, false, nil,
+		},
+		{
+			"not output to STDOUT, --no-tty set to false", false, false, false, true, testTTY,
+		},
+		{
+			"debug enabled, --no-tty flag is not used", true, false, false, false, nil,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			opts := &Terminal{
+				TTY:         testTTY,
+				noTTY:       tt.noTTY,
+				ttyEnforced: tt.ttyEnforced,
+			}
+			opts.DisableTTY(tt.debug, tt.toSTDOUT)
+			if !reflect.DeepEqual(opts.TTY, tt.expectedTTY) {
+				t.Fatalf("tt.TTY got %v, want %v", opts.TTY, tt.expectedTTY)
+			}
+		})
+	}
+}

cmd/oras/internal/option/terminal.go

@@ -41,6 +41,7 @@ type attachOptions struct {
 	option.Target
 	option.Format
 	option.Platform
+	option.Terminal
 
 	artifactType string
 	concurrency  int
@@ -97,6 +98,7 @@ Example - Attach file to the manifest tagged 'v1' in an OCI image layout folder
 			opts.FileRefs = args[1:]
 			err := option.Parse(cmd, &opts)
 			if err == nil {
+				opts.DisableTTY(opts.Debug, false)
 				if err = opts.EnsureReferenceNotEmpty(cmd, true); err == nil {
 					return nil
 				}

cmd/oras/internal/option/terminal_test.go

@@ -40,6 +40,7 @@ type fetchBlobOptions struct {
 	option.Descriptor
 	option.Pretty
 	option.Target
+	option.Terminal
 
 	outputPath string
 }
@@ -79,11 +80,11 @@ Example - Fetch and print a blob from OCI image layout archive file 'layout.tar'
 				return errors.New("`--output -` cannot be used with `--descriptor` at the same time")
 			}
 			opts.RawReference = args[0]
-			err := option.Parse(cmd, &opts)
-			if err == nil {
-				opts.UpdateTTY(cmd.Flags().Changed(option.NoTTYFlag), opts.outputPath == "-")
+			if err := option.Parse(cmd, &opts); err != nil {
+				return err
 			}
-			return err
+			opts.DisableTTY(opts.Debug, opts.outputPath == "-")
+			return nil
 		},
 		Aliases: []string{"get"},
 		RunE: func(cmd *cobra.Command, args []string) error {

cmd/oras/root/attach.go

@@ -53,7 +53,7 @@ func Test_fetchBlobOptions_doFetch(t *testing.T) {
 	}
 	var opts fetchBlobOptions
 	opts.Reference = tag
-	opts.Common.TTY = device
+	opts.TTY = device
 	opts.outputPath = t.TempDir() + "/test"
 	// test
 	_, err = opts.doFetch(ctx, src)