fix: resolve comments

Signed-off-by: Junjie Gao <junjiegao@microsoft.com>

cmd/notation/inspect_test.go

@@ -24,7 +24,7 @@ func TestInspectCommand_SecretsFromArgs(t *testing.T) {
 	opts := &inspectOpts{}
 	command := inspectCommand(opts)
 	format := option.Format{
-		FormatFlag: "text",
+		CurrentFormat: "text",
 	}
 	format.SetTypes(option.FormatTypeText, option.FormatTypeJSON)
 	expected := &inspectOpts{
@@ -60,7 +60,7 @@ func TestInspectCommand_SecretsFromEnv(t *testing.T) {
 
 	format := option.Format{}
 	format.SetTypes(option.FormatTypeText, option.FormatTypeJSON)
-	format.FormatFlag = "json"
+	format.CurrentFormat = "json"
 	expected := &inspectOpts{
 		reference: "ref",
 		SecureFlagOpts: SecureFlagOpts{

cmd/notation/internal/display/handler.go

@@ -27,11 +27,11 @@ import (
 
 // NewInpsectHandler creates a new InspectHandler based on the output format.
 func NewInpsectHandler(printer *output.Printer, format option.Format) (metadata.InspectHandler, error) {
-	switch option.FormatType(format.FormatFlag) {
+	switch option.FormatType(format.CurrentFormat) {
 	case option.FormatTypeJSON:
 		return json.NewInspectHandler(printer), nil
 	case option.FormatTypeText:
 		return tree.NewInspectHandler(printer), nil
 	}
-	return nil, fmt.Errorf("unrecognized output format %s", format.FormatFlag)
+	return nil, fmt.Errorf("unrecognized output format %s", format.CurrentFormat)
 }

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

@@ -11,6 +11,8 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+// Package metadata provides interfaces for handlers that render metadata
+// information for each command.
 package metadata
 
 import (

cmd/notation/internal/display/metadata/json/inspect.go

@@ -35,7 +35,7 @@ type inspectOutput struct {
 	Signatures []*signature `json:"signatures"`
 }
 
-// signature is the signature envelope for printing in human readable format.
+// signature is the signature envelope for printing in JSON format.
 type signature struct {
 	Digest                string             `json:"digest,omitempty"`
 	SignatureAlgorithm    string             `json:"signatureAlgorithm"`
@@ -46,16 +46,15 @@ type signature struct {
 	SignedArtifact        ocispec.Descriptor `json:"signedArtifact"`
 }
 
-// certificate is the certificate information for printing in human readable
-// format.
+// certificate is the certificate information for printing in JSON format.
 type certificate struct {
 	SHA256Fingerprint string    `json:"SHA256Fingerprint"`
 	IssuedTo          string    `json:"issuedTo"`
 	IssuedBy          string    `json:"issuedBy"`
 	Expiry            time.Time `json:"expiry"`
 }
 
-// timestamp is the timestamp information for printing in human readable.
+// timestamp is the timestamp information for printing in JSON format.
 type timestamp struct {
 	Timestamp    string         `json:"timestamp,omitempty"`
 	Certificates []*certificate `json:"certificates,omitempty"`
@@ -83,7 +82,7 @@ func NewInspectHandler(printer *output.Printer) *InspectHandler {
 // OnReferenceResolved sets the artifact reference and media type for the
 // handler.
 //
-// the reference is no-op for this handler.
+// The reference is no-op for this handler.
 func (h *InspectHandler) OnReferenceResolved(_, mediaType string) {
 	h.output.MediaType = mediaType
 }

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

@@ -18,6 +18,7 @@ import (
 	"crypto/x509"
 	"encoding/hex"
 	"fmt"
+	"maps"
 	"slices"
 	"strconv"
 	"strings"
@@ -121,7 +122,7 @@ func addUserDefinedAttributes(node *tree.Node, annotations map[string]string) {
 		userDefinedAttributesNode.Add("(empty)")
 		return
 	}
-	for _, k := range orderedKeys(annotations) {
+	for _, k := range slices.Sorted(maps.Keys(annotations)) {
 		v := annotations[k]
 		userDefinedAttributesNode.AddPair(k, v)
 	}
@@ -176,15 +177,6 @@ func addCertificates(node *tree.Node, certChain []*x509.Certificate) {
 	}
 }
 
-func orderedKeys(m map[string]string) []string {
-	keys := make([]string, 0, len(m))
-	for k := range m {
-		keys = append(keys, k)
-	}
-	slices.Sort(keys)
-	return keys
-}
-
 func formatTime(t time.Time) string {
 	return t.Format(time.ANSIC)
 }

cmd/notation/internal/option/format.go

@@ -30,6 +30,7 @@ package option
 
 import (
 	"fmt"
+	"slices"
 	"strings"
 
 	"github.com/spf13/cobra"
@@ -49,13 +50,13 @@ var (
 
 // Format contains input and parsed options for formatted output flags.
 type Format struct {
-	FormatFlag   string
-	allowedTypes []FormatType
+	CurrentFormat string
+	allowedTypes  []FormatType
 }
 
 // SetTypes sets the default format type and allowed format types.
 func (f *Format) SetTypes(defaultType FormatType, otherTypes ...FormatType) {
-	f.FormatFlag = string(defaultType)
+	f.CurrentFormat = string(defaultType)
 	f.allowedTypes = append(otherTypes, defaultType)
 }
 
@@ -67,16 +68,14 @@ func (f *Format) ApplyFlags(fs *pflag.FlagSet) {
 	}
 	usage := fmt.Sprintf("output format, options: %s", strings.Join(quotedAllowedTypes, ", "))
 	// apply flags
-	fs.StringVar(&f.FormatFlag, "output", f.FormatFlag, usage)
+	fs.StringVarP(&f.CurrentFormat, "output", "o", f.CurrentFormat, usage)
 }
 
 // Parse parses the input format flag.
 func (opts *Format) Parse(_ *cobra.Command) error {
-	for _, t := range opts.allowedTypes {
-		if opts.FormatFlag == string(t) {
-			// type validation passed
-			return nil
-		}
+	if ok := slices.Contains(opts.allowedTypes, FormatType(opts.CurrentFormat)); ok {
+		// type validation passed
+		return nil
 	}
-	return fmt.Errorf("invalid format type: %q", opts.FormatFlag)
+	return fmt.Errorf("invalid format type: %q", opts.CurrentFormat)
 }

cmd/notation/internal/output/print.go

@@ -29,6 +29,7 @@ limitations under the License.
 package output
 
 import (
+	"encoding/json"
 	"fmt"
 	"io"
 	"sync"
@@ -50,13 +51,15 @@ func NewPrinter(out io.Writer, err io.Writer) *Printer {
 func (p *Printer) Write(b []byte) (int, error) {
 	p.lock.Lock()
 	defer p.lock.Unlock()
+
 	return p.out.Write(b)
 }
 
 // Println prints objects concurrent-safely with newline.
 func (p *Printer) Println(a ...any) error {
 	p.lock.Lock()
 	defer p.lock.Unlock()
+
 	_, err := fmt.Fprintln(p.out, a...)
 	if err != nil {
 		err = fmt.Errorf("display output error: %w", err)
@@ -70,11 +73,19 @@ func (p *Printer) Println(a ...any) error {
 func (p *Printer) Printf(format string, a ...any) error {
 	p.lock.Lock()
 	defer p.lock.Unlock()
+
 	_, err := fmt.Fprintf(p.out, format, a...)
 	if err != nil {
 		err = fmt.Errorf("display output error: %w", err)
 		_, _ = fmt.Fprint(p.err, err)
+		return err
 	}
-	// Errors are handled above, so return nil
 	return nil
 }
+
+// PrintPrettyJSON prints object to out in JSON format.
+func PrintPrettyJSON(out io.Writer, object any) error {
+	encoder := json.NewEncoder(out)
+	encoder.SetIndent("", "  ")
+	return encoder.Encode(object)
+}

cmd/notation/internal/output/print_test.go

@@ -80,8 +80,22 @@ func TestPrinter_Print(t *testing.T) {
 		if mockWriter.errorCount != 2 {
 			t.Errorf("Expected two errors actual <%d>", mockWriter.errorCount)
 		}
-		if err != nil {
-			t.Error("Expected error to be ignored")
+		if err == nil {
+			t.Error("Expected error got <nil>")
 		}
 	})
 }
+
+func Test_PrintPrettyJSON(t *testing.T) {
+	builder := &strings.Builder{}
+	given := map[string]int{"bob": 5}
+	expected := "{\n  \"bob\": 5\n}\n"
+	err := PrintPrettyJSON(builder, given)
+	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 + ">")
+	}
+}

internal/cmd/flags.go

@@ -121,15 +121,6 @@ var (
 	SetPflagReferrersTag = func(fs *pflag.FlagSet, p *bool, usage string) {
 		fs.BoolVar(p, PflagReferrersTag.Name, true, usage)
 	}
-
-	PflagOutput = &pflag.Flag{
-		Name:      "output",
-		Shorthand: "o",
-	}
-	PflagOutputUsage = fmt.Sprintf("output format, options: '%s', '%s'", OutputJSON, OutputPlaintext)
-	SetPflagOutput   = func(fs *pflag.FlagSet, p *string, usage string) {
-		fs.StringVarP(p, PflagOutput.Name, PflagOutput.Shorthand, OutputPlaintext, usage)
-	}
 )
 
 // KeyValueSlice is a flag with type int

internal/ioutil/print.go

@@ -14,7 +14,6 @@
 package ioutil
 
 import (
-	"encoding/json"
 	"fmt"
 	"io"
 	"path/filepath"
@@ -79,15 +78,3 @@ func PrintCertMap(w io.Writer, certPaths []string) error {
 	}
 	return tw.Flush()
 }
-
-// PrintObjectAsJSON takes an interface and prints it as an indented JSON string
-func PrintObjectAsJSON(i interface{}) error {
-	jsonBytes, err := json.MarshalIndent(i, "", "    ")
-	if err != nil {
-		return err
-	}
-
-	fmt.Println(string(jsonBytes))
-
-	return nil
-}

test/e2e/suite/command/inspect.go

@@ -375,7 +375,7 @@ localhost:5000/e2e-inspect-invalid-timstamped@sha256:f1da8cd70d6d851fa2313c8d661
   ]
 }
 `
-			notation.Exec("inspect", "--output", "json", artifact.ReferenceWithDigest()).
+			notation.Exec("inspect", "-o", "json", artifact.ReferenceWithDigest()).
 				MatchContent(expectedOutput)
 		})
 	})