refactor print

Signed-off-by: Lixia (Sylvia) Lei <[email protected]>
oras-project · Nov 27, 2024 · e1a23a5 · e1a23a5
1 parent 6858781
commit e1a23a5
Show file tree

Hide file tree

Showing 14 changed files with 51 additions and 109 deletions.
diff --git a/cmd/oras/internal/display/handler_test.go b/cmd/oras/internal/display/handler_test.go
@@ -16,17 +16,18 @@ limitations under the License.
 package display
 
 import (
-	"oras.land/oras/internal/testutils"
 	"os"
 	"testing"
 
+	"oras.land/oras/internal/testutils"
+
 	"oras.land/oras/cmd/oras/internal/option"
 	"oras.land/oras/cmd/oras/internal/output"
 )
 
 func TestNewPushHandler(t *testing.T) {
 	mockFetcher := testutils.NewMockFetcher()
-	printer := output.NewPrinter(os.Stdout, os.Stderr, false)
+	printer := output.NewPrinter(os.Stdout, os.Stderr)
 	_, _, err := NewPushHandler(printer, option.Format{Type: option.FormatTypeText.Name}, os.Stdout, mockFetcher.Fetcher)
 	if err != nil {
 		t.Errorf("NewPushHandler() error = %v, want nil", err)
@@ -35,15 +36,15 @@ func TestNewPushHandler(t *testing.T) {
 
 func TestNewAttachHandler(t *testing.T) {
 	mockFetcher := testutils.NewMockFetcher()
-	printer := output.NewPrinter(os.Stdout, os.Stderr, false)
+	printer := output.NewPrinter(os.Stdout, os.Stderr)
 	_, _, err := NewAttachHandler(printer, option.Format{Type: option.FormatTypeText.Name}, os.Stdout, mockFetcher.Fetcher)
 	if err != nil {
 		t.Errorf("NewAttachHandler() error = %v, want nil", err)
 	}
 }
 
 func TestNewPullHandler(t *testing.T) {
-	printer := output.NewPrinter(os.Stdout, os.Stderr, false)
+	printer := output.NewPrinter(os.Stdout, os.Stderr)
 	_, _, err := NewPullHandler(printer, option.Format{Type: option.FormatTypeText.Name}, "", os.Stdout)
 	if err != nil {
 		t.Errorf("NewPullHandler() error = %v, want nil", err)

diff --git a/cmd/oras/internal/display/metadata/text/push_test.go b/cmd/oras/internal/display/metadata/text/push_test.go
@@ -65,7 +65,7 @@ func TestPushHandler_OnCompleted(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			printer := output.NewPrinter(tt.out, os.Stderr, false)
+			printer := output.NewPrinter(tt.out, os.Stderr)
 			p := &PushHandler{
 				printer: printer,
 			}

diff --git a/cmd/oras/internal/display/status/text.go b/cmd/oras/internal/display/status/text.go
@@ -49,7 +49,7 @@ func NewTextPushHandler(printer *output.Printer, fetcher content.Fetcher) PushHa
 
 // OnFileLoading is called when a file is being prepared for upload.
 func (ph *TextPushHandler) OnFileLoading(name string) error {
-	return ph.printer.PrintUntitled("Preparing", name)
+	return ph.printer.Println("Preparing", name)
 }
 
 // OnEmptyArtifact is called when an empty artifact is being uploaded.

diff --git a/cmd/oras/internal/display/status/text_test.go b/cmd/oras/internal/display/status/text_test.go
@@ -39,7 +39,7 @@ func TestMain(m *testing.M) {
 	mockFetcher = testutils.NewMockFetcher()
 	ctx = context.Background()
 	builder = &strings.Builder{}
-	printer = output.NewPrinter(builder, os.Stderr, false)
+	printer = output.NewPrinter(builder, os.Stderr)
 	bogus = ocispec.Descriptor{MediaType: ocispec.MediaTypeImageManifest}
 	os.Exit(m.Run())
 }
@@ -204,14 +204,14 @@ func TestTextManifestIndexUpdateHandler_OnManifestAdded(t *testing.T) {
 	}{
 		{
 			name:    "ref is a digest",
-			printer: output.NewPrinter(os.Stdout, os.Stderr, false),
+			printer: output.NewPrinter(os.Stdout, os.Stderr),
 			ref:     "sha256:fd6ed2f36b5465244d5dc86cb4e7df0ab8a9d24adc57825099f522fe009a22bb",
 			desc:    ocispec.Descriptor{MediaType: "test", Digest: "sha256:fd6ed2f36b5465244d5dc86cb4e7df0ab8a9d24adc57825099f522fe009a22bb", Size: 25},
 			wantErr: false,
 		},
 		{
 			name:    "ref is not a digest",
-			printer: output.NewPrinter(os.Stdout, os.Stderr, false),
+			printer: output.NewPrinter(os.Stdout, os.Stderr),
 			ref:     "v1",
 			desc:    ocispec.Descriptor{MediaType: "test", Digest: "sha256:fd6ed2f36b5465244d5dc86cb4e7df0ab8a9d24adc57825099f522fe009a22bb", Size: 25},
 			wantErr: false,
@@ -239,14 +239,14 @@ func TestTextManifestIndexUpdateHandler_OnIndexMerged(t *testing.T) {
 	}{
 		{
 			name:    "ref is a digest",
-			printer: output.NewPrinter(os.Stdout, os.Stderr, false),
+			printer: output.NewPrinter(os.Stdout, os.Stderr),
 			ref:     "sha256:fd6ed2f36b5465244d5dc86cb4e7df0ab8a9d24adc57825099f522fe009a22bb",
 			desc:    ocispec.Descriptor{MediaType: "test", Digest: "sha256:fd6ed2f36b5465244d5dc86cb4e7df0ab8a9d24adc57825099f522fe009a22bb", Size: 25},
 			wantErr: false,
 		},
 		{
 			name:    "ref is not a digest",
-			printer: output.NewPrinter(os.Stdout, os.Stderr, false),
+			printer: output.NewPrinter(os.Stdout, os.Stderr),
 			ref:     "v1",
 			desc:    ocispec.Descriptor{MediaType: "test", Digest: "sha256:fd6ed2f36b5465244d5dc86cb4e7df0ab8a9d24adc57825099f522fe009a22bb", Size: 25},
 			wantErr: false,

diff --git a/cmd/oras/internal/option/common.go b/cmd/oras/internal/option/common.go
@@ -35,10 +35,7 @@ type Common struct {
 	// no longer takes effect.
 	Verbose bool
 
-	// SuppressUntitled suppresses the status output for untitled blobs.
-	// Default value: false.
-	SuppressUntitled bool
-	TTY              *os.File
+	TTY *os.File
 	*output.Printer
 }
 
@@ -53,7 +50,7 @@ func (opts *Common) ApplyFlags(fs *pflag.FlagSet) {
 
 // Parse gets target options from user input.
 func (opts *Common) Parse(cmd *cobra.Command) error {
-	opts.Printer = output.NewPrinter(cmd.OutOrStdout(), cmd.OutOrStderr(), opts.SuppressUntitled)
+	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)
 }

diff --git a/cmd/oras/internal/output/print.go b/cmd/oras/internal/output/print.go
@@ -27,15 +27,14 @@ import (
 
 // Printer prints for status handlers.
 type Printer struct {
-	out              io.Writer
-	err              io.Writer
-	lock             sync.Mutex
-	suppressUntitled bool
+	out  io.Writer
+	err  io.Writer
+	lock sync.Mutex
 }
 
 // NewPrinter creates a new Printer.
-func NewPrinter(out io.Writer, err io.Writer, suppressUntitled bool) *Printer {
-	return &Printer{out: out, err: err, suppressUntitled: suppressUntitled}
+func NewPrinter(out io.Writer, err io.Writer) *Printer {
+	return &Printer{out: out, err: err}
 }
 
 // Write implements the io.Writer interface.
@@ -71,19 +70,15 @@ func (p *Printer) Printf(format string, a ...any) error {
 	return nil
 }
 
-// PrintUntitled prints unless suppressed.
-func (p *Printer) PrintUntitled(a ...any) error {
-	if p.suppressUntitled {
-		return nil
-	}
-	return p.Println(a...)
-}
-
 // PrintStatus prints transfer status.
 func (p *Printer) PrintStatus(desc ocispec.Descriptor, status string) error {
-	name, isTitle := descriptor.GetTitleOrMediaType(desc)
-	if !isTitle {
-		return p.PrintUntitled(status, descriptor.ShortDigest(desc), name)
+	return p.Println(status, descriptor.ShortDigest(desc), descriptor.GetName(desc))
+}
+
+// PrintStatusUnlessSuppressed prints transfer status unless suppressed.
+func (p *Printer) PrintStatusUnlessSuppressed(desc ocispec.Descriptor, status string, suppressed bool) error {
+	if suppressed {
+		return nil
 	}
-	return p.Println(status, descriptor.ShortDigest(desc), name)
+	return p.PrintStatus(desc, status)
 }
diff --git a/cmd/oras/internal/output/print_test.go b/cmd/oras/internal/output/print_test.go
@@ -43,7 +43,7 @@ func (mw *mockWriter) String() string {
 
 func TestPrinter_Println(t *testing.T) {
 	mockWriter := &mockWriter{}
-	printer := NewPrinter(mockWriter, os.Stderr, false)
+	printer := NewPrinter(mockWriter, os.Stderr)
 	err := printer.Println("boom")
 	if mockWriter.errorCount != 1 {
 		t.Error("Expected one error actual <" + strconv.Itoa(mockWriter.errorCount) + ">")
@@ -59,47 +59,3 @@ func TestPrinter_Println(t *testing.T) {
 		t.Error("Expected error to be ignored")
 	}
 }
-
-func TestPrinter_SuppressUntitled(t *testing.T) {
-	builder := &strings.Builder{}
-	suppressUntitled := true
-	printer := NewPrinter(builder, os.Stderr, suppressUntitled)
-
-	expected := "normal\nthing one\n"
-	err := printer.Println("normal")
-	if err != nil {
-		t.Error("Expected no error got <" + err.Error() + ">")
-	}
-	err = printer.Printf("thing %s\n", "one")
-	if err != nil {
-		t.Error("Expected no error got <" + err.Error() + ">")
-	}
-	err = printer.PrintUntitled("verbose")
-	if err != nil {
-		t.Error("Expected no error got <" + err.Error() + ">")
-	}
-	actual := builder.String()
-	if expected != actual {
-		t.Error("Expected <" + expected + "> not equal to actual <" + actual + ">")
-	}
-}
-
-func TestPrinter_PrintUntitled(t *testing.T) {
-	builder := &strings.Builder{}
-	suppressUntitled := false
-	printer := NewPrinter(builder, os.Stderr, suppressUntitled)
-
-	expected := "normal\nverbose\n"
-	err := printer.Println("normal")
-	if err != nil {
-		t.Error("Expected no error got <" + err.Error() + ">")
-	}
-	err = printer.PrintUntitled("verbose")
-	if err != nil {
-		t.Error("Expected no error got <" + err.Error() + ">")
-	}
-	actual := builder.String()
-	if expected != actual {
-		t.Error("Expected <" + expected + "> not equal to actual <" + actual + ">")
-	}
-}
diff --git a/cmd/oras/root/blob/push.go b/cmd/oras/root/blob/push.go
@@ -87,7 +87,6 @@ Example - Push blob 'hi.txt' into an OCI image layout folder 'layout-dir':
 					return errors.New("`--size` must be provided if the blob is read from stdin")
 				}
 			}
-			opts.SuppressUntitled = opts.OutputDescriptor
 			return option.Parse(cmd, &opts)
 		},
 		RunE: func(cmd *cobra.Command, args []string) error {
@@ -121,9 +120,9 @@ func pushBlob(cmd *cobra.Command, opts *pushBlobOptions) (err error) {
 		return err
 	}
 	if exists {
-		err = opts.PrintStatus(desc, "Exists")
+		err = opts.PrintStatusUnlessSuppressed(desc, "Exists", opts.OutputDescriptor)
 	} else {
-		err = opts.doPush(ctx, opts.Printer, target, desc, rc)
+		err = opts.doPush(ctx, opts.Printer, target, desc, rc, opts.OutputDescriptor)
 	}
 	if err != nil {
 		return err
@@ -142,16 +141,16 @@ func pushBlob(cmd *cobra.Command, opts *pushBlobOptions) (err error) {
 
 	return nil
 }
-func (opts *pushBlobOptions) doPush(ctx context.Context, printer *output.Printer, t oras.Target, desc ocispec.Descriptor, r io.Reader) error {
+func (opts *pushBlobOptions) doPush(ctx context.Context, printer *output.Printer, t oras.Target, desc ocispec.Descriptor, r io.Reader, statusSuppressed bool) error {
 	if opts.TTY == nil {
 		// none TTY output
-		if err := printer.PrintStatus(desc, "Uploading"); err != nil {
+		if err := printer.PrintStatusUnlessSuppressed(desc, "Uploading", statusSuppressed); err != nil {
 			return err
 		}
 		if err := t.Push(ctx, desc, r); err != nil {
 			return err
 		}
-		return printer.PrintStatus(desc, "Uploaded ")
+		return printer.PrintStatusUnlessSuppressed(desc, "Uploaded ", statusSuppressed)
 	}
 
 	// TTY output

diff --git a/cmd/oras/root/blob/push_test.go b/cmd/oras/root/blob/push_test.go
@@ -20,10 +20,11 @@ package blob
 import (
 	"bytes"
 	"context"
-	"oras.land/oras/cmd/oras/internal/output"
 	"os"
 	"testing"
 
+	"oras.land/oras/cmd/oras/internal/output"
+
 	"github.com/opencontainers/go-digest"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 	"oras.land/oras-go/v2/content/memory"
@@ -40,7 +41,7 @@ func Test_pushBlobOptions_doPush(t *testing.T) {
 	src := memory.New()
 	content := []byte("test")
 	r := bytes.NewReader(content)
-	printer := output.NewPrinter(os.Stdout, os.Stderr, false)
+	printer := output.NewPrinter(os.Stdout, os.Stderr)
 	desc := ocispec.Descriptor{
 		MediaType: "application/octet-stream",
 		Digest:    digest.FromBytes(content),
@@ -49,7 +50,7 @@ func Test_pushBlobOptions_doPush(t *testing.T) {
 	var opts pushBlobOptions
 	opts.Common.TTY = device
 	// test
-	err = opts.doPush(context.Background(), printer, src, desc, r)
+	err = opts.doPush(context.Background(), printer, src, desc, r, false)
 	if err != nil {
 		t.Fatal(err)
 	}

diff --git a/cmd/oras/root/cp_test.go b/cmd/oras/root/cp_test.go
@@ -133,7 +133,7 @@ func Test_doCopy(t *testing.T) {
 	opts.From.Reference = memDesc.Digest.String()
 	dst := memory.New()
 	builder := &strings.Builder{}
-	printer := output.NewPrinter(builder, os.Stderr, opts.SuppressUntitled)
+	printer := output.NewPrinter(builder, os.Stderr)
 	handler := status.NewTextCopyHandler(printer, dst)
 	// test
 	_, err = doCopy(context.Background(), handler, memStore, dst, &opts)
@@ -157,7 +157,7 @@ func Test_doCopy_skipped(t *testing.T) {
 	opts.TTY = slave
 	opts.From.Reference = memDesc.Digest.String()
 	builder := &strings.Builder{}
-	printer := output.NewPrinter(builder, os.Stderr, opts.SuppressUntitled)
+	printer := output.NewPrinter(builder, os.Stderr)
 	handler := status.NewTextCopyHandler(printer, memStore)
 
 	// test
@@ -193,7 +193,7 @@ func Test_doCopy_mounted(t *testing.T) {
 	}
 	to.PlainHTTP = true
 	builder := &strings.Builder{}
-	printer := output.NewPrinter(builder, os.Stderr, opts.SuppressUntitled)
+	printer := output.NewPrinter(builder, os.Stderr)
 	handler := status.NewTextCopyHandler(printer, to)
 
 	// test

diff --git a/cmd/oras/root/manifest/push.go b/cmd/oras/root/manifest/push.go
@@ -96,7 +96,6 @@ Example - Push a manifest to an OCI image layout folder 'layout-dir' and tag wit
 			refs := strings.Split(args[0], ",")
 			opts.RawReference = refs[0]
 			opts.extraRefs = refs[1:]
-			opts.SuppressUntitled = opts.OutputDescriptor
 			return option.Parse(cmd, &opts)
 		},
 		RunE: func(cmd *cobra.Command, args []string) error {
@@ -157,17 +156,17 @@ func pushManifest(cmd *cobra.Command, opts pushOptions) error {
 		return err
 	}
 	if match {
-		if err := opts.PrintStatus(desc, "Exists"); err != nil {
+		if err := opts.PrintStatusUnlessSuppressed(desc, "Exists", opts.OutputDescriptor); err != nil {
 			return err
 		}
 	} else {
-		if err = opts.PrintStatus(desc, "Uploading"); err != nil {
+		if err = opts.PrintStatusUnlessSuppressed(desc, "Uploading", opts.OutputDescriptor); err != nil {
 			return err
 		}
 		if _, err := oras.TagBytes(ctx, target, mediaType, contentBytes, ref); err != nil {
 			return err
 		}
-		if err = opts.PrintStatus(desc, "Uploaded "); err != nil {
+		if err = opts.PrintStatusUnlessSuppressed(desc, "Uploaded ", opts.OutputDescriptor); err != nil {
 			return err
 		}
 	}

diff --git a/cmd/oras/root/version.go b/cmd/oras/root/version.go
@@ -46,7 +46,7 @@ Example - print version:
 			return nil
 		},
 		RunE: func(cmd *cobra.Command, args []string) error {
-			printer := output.NewPrinter(cmd.OutOrStdout(), cmd.ErrOrStderr(), false)
+			printer := output.NewPrinter(cmd.OutOrStdout(), cmd.ErrOrStderr())
 			return runVersion(printer)
 		},
 	}

diff --git a/internal/descriptor/descriptor.go b/internal/descriptor/descriptor.go
@@ -66,13 +66,13 @@ func Plain(desc ocispec.Descriptor) ocispec.Descriptor {
 	}
 }
 
-// GetTitleOrMediaType gets a descriptor name using either title or media type.
-func GetTitleOrMediaType(desc ocispec.Descriptor) (name string, isTitle bool) {
-	name, ok := desc.Annotations[ocispec.AnnotationTitle]
+// GetName gets a descriptor name using either title or media type.
+func GetName(desc ocispec.Descriptor) string {
+	title, ok := desc.Annotations[ocispec.AnnotationTitle]
 	if !ok {
-		return desc.MediaType, false
+		return desc.MediaType
 	}
-	return name, true
+	return title
 }
 
 // GenerateContentKey generates a unique key for each content descriptor using