fix: resolve comments

Signed-off-by: Junjie Gao <[email protected]>

cmd/notation/internal/display/metadata/interface.go

@@ -81,10 +81,11 @@ type BlobVerifyHandler interface {
 // signatures.
 type ListHandler interface {
 	Renderer
+
 	// OnReferenceResolved sets the artifact reference and media type for the
 	// handler.
 	OnReferenceResolved(reference string)
 
 	// OnSignatureListed adds the signature digest to be rendered.
-	OnSignatureListed(signatureManifest ocispec.Descriptor)
+	OnSignatureListed(signatureManifest ocispec.Descriptor) error
 }

cmd/notation/internal/display/metadata/tree/list.go

@@ -26,7 +26,7 @@ type ListHandler struct {
 
 	// sprinter is a streaming printer to print the signature digest nodes in
 	// a streaming fashion
-	sprinter *streamingPrinter
+	sprinter *StreamPrinter
 
 	// headerNode contains the headers of the output
 	//
@@ -43,7 +43,7 @@ type ListHandler struct {
 func NewListHandler(printer *output.Printer) *ListHandler {
 	return &ListHandler{
 		printer:  printer,
-		sprinter: newStreamingPrinter("    ", printer),
+		sprinter: newStreamPrinter("    ", printer),
 	}
 }
 
@@ -55,20 +55,24 @@ func (h *ListHandler) OnReferenceResolved(reference string) {
 }
 
 // OnSignatureListed adds the signature digest to be printed.
-func (h *ListHandler) OnSignatureListed(signatureManifest ocispec.Descriptor) {
+func (h *ListHandler) OnSignatureListed(signatureManifest ocispec.Descriptor) error {
 	// print the header
 	if !h.headerPrinted {
-		h.headerNode.Print(h.printer)
+		if err := h.headerNode.Print(h.printer); err != nil {
+			return err
+		}
 		h.headerPrinted = true
 	}
-	h.sprinter.PrintNode(newNode(signatureManifest.Digest.String()))
+	return h.sprinter.PrintNode(newNode(signatureManifest.Digest.String()))
 }
 
 // Render completes the rendering of the list of signature digests.
 func (h *ListHandler) Render() error {
-	if h.sprinter.prevNode == nil {
+	if err := h.sprinter.Flush(); err != nil {
+		return err
+	}
+	if !h.headerPrinted {
 		return h.printer.Printf("%s has no associated signatures\n", h.headerNode.Value)
 	}
-	h.sprinter.Complete()
 	return nil
 }

cmd/notation/internal/display/metadata/tree/printer.go

@@ -15,39 +15,42 @@ package tree
 
 import "io"
 
-// streamingPrinter prints the tree nodes in a streaming fashion.
-type streamingPrinter struct {
+// StreamPrinter prints the tree nodes in a streaming fashion.
+type StreamPrinter struct {
 	w        io.Writer
 	prefix   string
 	prevNode *node
 }
 
-// newStreamingPrinter creates a new streaming printer.
+// newStreamPrinter creates a new stream printer.
 //
 // prefix is the prefix string that will be inherited by the nodes that are
 // printed.
-func newStreamingPrinter(prefix string, w io.Writer) *streamingPrinter {
-	return &streamingPrinter{
+func newStreamPrinter(prefix string, w io.Writer) *StreamPrinter {
+	return &StreamPrinter{
 		w:      w,
 		prefix: prefix,
 	}
 }
 
 // PrintNode adds a new node to be ready to print.
-func (p *streamingPrinter) PrintNode(node *node) {
+func (p *StreamPrinter) PrintNode(node *node) error {
 	if p.prevNode == nil {
 		p.prevNode = node
-		return
+		return nil
+	}
+	if err := print(p.w, p.prefix, treeItemPrefix, p.prefix+subTreePrefix, p.prevNode); err != nil {
+		return err
 	}
-	print(p.w, p.prefix, treeItemPrefix, p.prefix+subTreePrefix, p.prevNode)
 	p.prevNode = node
+	return nil
 }
 
-// Complete prints the last node and completes the printing.
-func (p *streamingPrinter) Complete() {
+// Flush prints the last node and completes the printing.
+func (p *StreamPrinter) Flush() error {
 	if p.prevNode != nil {
 		// print the last node
-		print(p.w, p.prefix, treeItemPrefixLast, p.prefix+subTreePrefixLast, p.prevNode)
-		p.prevNode = nil
+		return print(p.w, p.prefix, treeItemPrefixLast, p.prefix+subTreePrefixLast, p.prevNode)
 	}
+	return nil
 }

cmd/notation/internal/display/metadata/tree/printer_test.go

@@ -22,8 +22,8 @@ func TestStreamingPrinter(t *testing.T) {
 	t.Run("empty output", func(t *testing.T) {
 		expected := ""
 		buff := &bytes.Buffer{}
-		p := newStreamingPrinter("", buff)
-		p.Complete()
+		p := newStreamPrinter("", buff)
+		p.Flush()
 
 		if buff.String() != expected {
 			t.Fatalf("expected %s, got %s", expected, buff.String())
@@ -33,9 +33,9 @@ func TestStreamingPrinter(t *testing.T) {
 	t.Run("one node", func(t *testing.T) {
 		expected := "└── a\n"
 		buff := &bytes.Buffer{}
-		p := newStreamingPrinter("", buff)
+		p := newStreamPrinter("", buff)
 		p.PrintNode(newNode("a"))
-		p.Complete()
+		p.Flush()
 
 		if buff.String() != expected {
 			t.Fatalf("expected %s, got %s", expected, buff.String())
@@ -47,10 +47,10 @@ func TestStreamingPrinter(t *testing.T) {
 └── b
 `
 		buff := &bytes.Buffer{}
-		p := newStreamingPrinter("", buff)
+		p := newStreamPrinter("", buff)
 		p.PrintNode(newNode("a"))
 		p.PrintNode(newNode("b"))
-		p.Complete()
+		p.Flush()
 
 		if buff.String() != expected {
 			t.Fatalf("expected %s, got %s", expected, buff.String())
@@ -68,7 +68,7 @@ func TestStreamingPrinter(t *testing.T) {
     └── h
 `
 		buff := &bytes.Buffer{}
-		p := newStreamingPrinter("", buff)
+		p := newStreamPrinter("", buff)
 		// create the tree
 		a := newNode("a")
 		b := a.Add("b")
@@ -82,7 +82,7 @@ func TestStreamingPrinter(t *testing.T) {
 		e.Add("h")
 		p.PrintNode(e)
 
-		p.Complete()
+		p.Flush()
 
 		if buff.String() != expected {
 			t.Fatalf("expected %s, got %s", expected, buff.String())
@@ -100,7 +100,7 @@ func TestStreamingPrinter(t *testing.T) {
     │       └── h
 `
 		buff := &bytes.Buffer{}
-		p := newStreamingPrinter("    │   ", buff)
+		p := newStreamPrinter("    │   ", buff)
 		// create the tree
 		a := newNode("a")
 		b := a.Add("b")
@@ -114,7 +114,7 @@ func TestStreamingPrinter(t *testing.T) {
 		e.Add("h")
 		p.PrintNode(e)
 
-		p.Complete()
+		p.Flush()
 
 		if buff.String() != expected {
 			t.Fatalf("expected %s, got %s", expected, buff.String())

cmd/notation/list.go

@@ -121,10 +121,7 @@ func runList(ctx context.Context, opts *listOpts) error {
 	displayHandler.OnReferenceResolved(resolvedRef)
 
 	// list signatures
-	if err := listSignatures(ctx, sigRepo, manifestDesc, opts.maxSignatures, func(sigManifestDesc ocispec.Descriptor) error {
-		displayHandler.OnSignatureListed(sigManifestDesc)
-		return nil
-	}); err != nil {
+	if err := listSignatures(ctx, sigRepo, manifestDesc, opts.maxSignatures, displayHandler.OnSignatureListed); err != nil {
 		var errExceedMaxSignatures cmderr.ErrorExceedMaxSignatures
 		if !errors.As(err, &errExceedMaxSignatures) {
 			return err

test/e2e/suite/command/list.go

@@ -163,11 +163,15 @@ var _ = Describe("notation list", func() {
 			artifact := GenerateArtifact("e2e-valid-multiple-signatures", "")
 
 			notation.Exec("list", artifact.ReferenceWithDigest()).
-				MatchContent(artifact.ReferenceWithDigest() + `
-└── application/vnd.cncf.notary.signature
-    ├── sha256:c3ebe4a20b6832328fc5078a7795ddc1114b896e13fca2add38109c3866b5fbf
-    └── sha256:90ceaff260d657d797c408ac73564a9c7bb9d86055877c2a811f0e63b8c6524f
-`)
+				MatchKeyWords(
+					artifact.ReferenceWithDigest(),
+					// the order of the signatures is not guaranteed
+					"└── application/vnd.cncf.notary.signature",
+					"    ├── ",
+					"    └── ",
+					"sha256:c3ebe4a20b6832328fc5078a7795ddc1114b896e13fca2add38109c3866b5fbf",
+					"sha256:90ceaff260d657d797c408ac73564a9c7bb9d86055877c2a811f0e63b8c6524f",
+				)
 		})
 	})