chore: standardize quoting in error and log messages

cmd/deploy.go

@@ -118,7 +118,7 @@ func (c *deployCommander) getPlanFilter(logger *logging.Logger) *pkg.NlxPlanFilt
 	/*
 		if len(c.pages) > 0 {
 			logEntry.Data["pages"] = c.pages
-			logEntry.Infof("Filtering retrieval to pages: %q", c.pages)
+			logEntry.Infof("Filtering retrieval to pages: %v", c.pages)
 			filter.PageNames = c.pages
 		}
 	*/

cmd/retrieve.go

@@ -103,7 +103,7 @@ func (c *retrieveCommander) getPlanFilter(logger *logging.Logger) *pkg.NlxPlanFi
 	/*
 		if len(c.pages) > 0 {
 			initFilter()
-			logger.Infof("Filtering retrieval to pages: %q", c.pages)
+			logger.Infof("Filtering retrieval to pages: %v", c.pages)
 			filter.PageNames = c.pages
 		}
 	*/

pkg/cmdutil/commander.go

@@ -52,7 +52,7 @@ func applyEnvVar(cmd *cobra.Command, pf *pflag.Flag) (*AppliedEnvVar, error) {
 		// MUST use flagSet.Set to ensure that flag.Changed is updated
 		// calling flag.Value.Set will only set the value
 		if errSet := cmd.Flags().Set(pf.Name, envVal); errSet != nil {
-			return nil, fmt.Errorf("unable to use value from environment variable %v for flag %q: %v", envVarName, pf.Name, errSet)
+			return nil, fmt.Errorf("unable to use value from environment variable %v for flag --%v: %v", envVarName, pf.Name, errSet)
 		}
 
 		// cmd.Flags().Lookup(pf.Name).Value could be used to log the value of the flag at this point

pkg/cmdutil/commander_test.go

@@ -208,7 +208,7 @@ func (suite *ApplyEnvVarsTestSuite) TestFailsToUpdate() {
 
 	err := testutil.ExecuteCommand(cmd, []string{}...)
 
-	assert.ErrorContains(t, err, fmt.Sprintf("unable to use value from environment variable %v for flag %q", "SKUID_FOO", f.Name))
+	assert.ErrorContains(t, err, fmt.Sprintf("unable to use value from environment variable %v for flag --%v", "SKUID_FOO", f.Name))
 	assert.Equal(t, len(appliedEnvVars), 0)
 }
 

pkg/cmdutil/flags.go

@@ -106,17 +106,17 @@ func setAnnotations[T flags.FlagType | ~[]F, F flags.FlagType](fs *pflag.FlagSet
 
 func checkValidFlag[T flags.FlagType | ~[]F, F flags.FlagType](f *flags.Flag[T, F], allowParse bool, allowRedact bool) {
 	// should never happen in production
-	errutil.MustConditionf(flagNameValidator.MatchString(f.Name), "flag name %q is invalid", f.Name)
-	errutil.MustConditionf(len(f.Usage) >= 10, "flag usage %q is invalid for flag name %q", f.Usage, f.Name)
+	errutil.MustConditionf(flagNameValidator.MatchString(f.Name), "flag name %v is invalid", f.Name)
+	errutil.MustConditionf(len(f.Usage) >= 10, "flag usage %v is invalid for flag name %v", logging.QuoteText(f.Usage), f.Name)
 	if allowParse {
-		errutil.MustConditionf(f.Parse != nil, "flag type %T requires Parse to be defined for flag name %q", f, f.Name)
+		errutil.MustConditionf(f.Parse != nil, "flag type %T requires Parse to be defined for flag name %v", f, f.Name)
 	} else {
-		errutil.MustConditionf(f.Parse == nil, "flag type %T does not support Parse for flag name %q", f, f.Name)
+		errutil.MustConditionf(f.Parse == nil, "flag type %T does not support Parse for flag name %v", f, f.Name)
 	}
 	if allowRedact {
-		errutil.MustConditionf(f.Redact != nil, "flag type %T requires Redact to be defined for flag name %q", f, f.Name)
+		errutil.MustConditionf(f.Redact != nil, "flag type %T requires Redact to be defined for flag name %v", f, f.Name)
 	} else {
-		errutil.MustConditionf(f.Redact == nil, "flag type %T does not support Redact for flag name %q", f, f.Name)
+		errutil.MustConditionf(f.Redact == nil, "flag type %T does not support Redact for flag name %v", f, f.Name)
 	}
 	// due to limitations of Go generics, it is possible to define a flags.Flag that has mismatched types
 	// for T & F when T is not a slice.  The below will validate that T & F point to the same type when T
@@ -129,7 +129,7 @@ func checkValidFlag[T flags.FlagType | ~[]F, F flags.FlagType](f *flags.Flag[T,
 	tType := reflect.TypeOf(*new(T))
 	if tType.Kind() != reflect.Slice {
 		fType := reflect.TypeOf(*new(F))
-		errutil.MustConditionf(tType == fType, "type parameters must be the same type for flag name %q", f.Name)
+		errutil.MustConditionf(tType == fType, "type parameters must be the same type for flag name %v", f.Name)
 	}
 }
 

pkg/cmdutil/flags_test.go

@@ -7,6 +7,7 @@ import (
 	"github.com/skuid/skuid-cli/pkg"
 	"github.com/skuid/skuid-cli/pkg/cmdutil"
 	"github.com/skuid/skuid-cli/pkg/flags"
+	"github.com/skuid/skuid-cli/pkg/logging"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 	"github.com/stretchr/testify/assert"
@@ -138,27 +139,27 @@ func TestEnvVarName(t *testing.T) {
 }
 
 func expectInvalidFlagNameError(name string) error {
-	return fmt.Errorf("flag name %q is invalid", name)
+	return fmt.Errorf("flag name %v is invalid", name)
 }
 
 func expectInvalidUsageError(name string, usage string) error {
-	return fmt.Errorf("flag usage %q is invalid for flag name %q", usage, name)
+	return fmt.Errorf("flag usage %v is invalid for flag name %v", logging.QuoteText(usage), name)
 }
 
 func expectInvalidParseError[T flags.FlagType | ~[]F, F flags.FlagType](name string) error {
-	return fmt.Errorf("flag type %T does not support Parse for flag name %q", new(flags.Flag[T, F]), name)
+	return fmt.Errorf("flag type %T does not support Parse for flag name %v", new(flags.Flag[T, F]), name)
 }
 
 func expectMissingParseError[T flags.FlagType | ~[]F, F flags.FlagType](name string) error {
-	return fmt.Errorf("flag type %T requires Parse to be defined for flag name %q", new(flags.Flag[T, F]), name)
+	return fmt.Errorf("flag type %T requires Parse to be defined for flag name %v", new(flags.Flag[T, F]), name)
 }
 
 func expectInvalidRedactError[T flags.FlagType | ~[]F, F flags.FlagType](name string) error {
-	return fmt.Errorf("flag type %T does not support Redact for flag name %q", new(flags.Flag[T, F]), name)
+	return fmt.Errorf("flag type %T does not support Redact for flag name %v", new(flags.Flag[T, F]), name)
 }
 
 func expectMissingRedactError[T flags.FlagType | ~[]F, F flags.FlagType](name string) error {
-	return fmt.Errorf("flag type %T requires Redact to be defined for flag name %q", new(flags.Flag[T, F]), name)
+	return fmt.Errorf("flag type %T requires Redact to be defined for flag name %v", new(flags.Flag[T, F]), name)
 }
 
 func emptyParse[F flags.FlagType](val string) (F, error) {

pkg/flags/parsers.go

@@ -9,6 +9,7 @@ import (
 	"time"
 
 	"github.com/sirupsen/logrus"
+	"github.com/skuid/skuid-cli/pkg/logging"
 	"github.com/skuid/skuid-cli/pkg/metadata"
 	"github.com/skuid/skuid-cli/pkg/util"
 )
@@ -96,7 +97,7 @@ func ParseString(options ParseStringOptions) func(val string) (string, error) {
 func ParseLogLevel(val string) (logrus.Level, error) {
 	// simple wrapper to customize error message, otherwise it will contain the word logrus
 	if l, err := logrus.ParseLevel(strings.TrimSpace(val)); err != nil {
-		return l, fmt.Errorf("not a valid log level: %q", val)
+		return l, fmt.Errorf("not a valid log level: %v", logging.QuoteText(val))
 	} else {
 		return l, nil
 	}
@@ -112,7 +113,7 @@ func ParseMetadataEntity(val string) (metadata.MetadataEntity, error) {
 
 func ParseDirectory(val string) (string, error) {
 	if targetDirectory, err := filepath.Abs(val); err != nil {
-		return "", fmt.Errorf("unable to convert %q to absolute path: %w", val, err)
+		return "", fmt.Errorf("unable to convert %v to absolute path: %w", logging.QuoteText(val), err)
 	} else {
 		return targetDirectory, nil
 	}

pkg/flags/parsers_test.go

@@ -10,6 +10,7 @@ import (
 
 	"github.com/sirupsen/logrus"
 	"github.com/skuid/skuid-cli/pkg/flags"
+	"github.com/skuid/skuid-cli/pkg/logging"
 	"github.com/skuid/skuid-cli/pkg/metadata"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -251,7 +252,7 @@ func TestParseString(t *testing.T) {
 
 func TestParseLogLevel(t *testing.T) {
 	expectError := func(val string) error {
-		return fmt.Errorf("not a valid log level: %q", val)
+		return fmt.Errorf("not a valid log level: %v", logging.QuoteText(val))
 	}
 
 	testCases := []struct {

pkg/metadata/metadata.go

@@ -52,7 +52,7 @@ const (
 	MetadataPathTypeEntityFile
 
 	EntityNameSite                     string = "site"
-	metadataTypeEntityPathNotSupported string = "metadata type %q does not support the entity path: %q"
+	metadataTypeEntityPathNotSupported string = "metadata type %v does not support the entity path: %v"
 	AnyValue                           string = "*"
 )
 
@@ -281,7 +281,7 @@ func (from NlxMetadata) GetFieldValue(target MetadataType) []string {
 	value := reflect.ValueOf(from)
 	field := value.FieldByName(name)
 	// should never occur in production
-	errutil.MustConditionf(field.IsValid(), "unable to locate metadata field for metadata type %q", name)
+	errutil.MustConditionf(field.IsValid(), "unable to locate metadata field for metadata type %v", name)
 	return field.Interface().([]string)
 }
 
@@ -317,7 +317,7 @@ func NewMetadataEntity(entityPath string) (*MetadataEntity, error) {
 	}
 	valid := validateEntityName(details)
 	if !valid {
-		return nil, NewMetadataPathError(entityPath, MetadataPathTypeEntity, fmt.Errorf(metadataTypeEntityPathNotSupported, details.Type.Name(), entityPath))
+		return nil, NewMetadataPathError(entityPath, MetadataPathTypeEntity, fmt.Errorf(metadataTypeEntityPathNotSupported, details.Type.Name(), logging.QuoteText(entityPath)))
 	}
 	entity := MetadataEntity{
 		Type:         details.Type,
@@ -338,13 +338,13 @@ func NewMetadataEntityFile(entityFilePath string) (*MetadataEntityFile, error) {
 	}
 	entityName, entityRelativePath, isEntityDefinitionFile, valid := entityNameFromFilePath(details)
 	if !valid {
-		return nil, NewMetadataPathError(entityFilePath, MetadataPathTypeEntityFile, fmt.Errorf(metadataTypeEntityPathNotSupported, details.Type.Name(), entityFilePath))
+		return nil, NewMetadataPathError(entityFilePath, MetadataPathTypeEntityFile, fmt.Errorf(metadataTypeEntityPathNotSupported, details.Type.Name(), logging.QuoteText(entityFilePath)))
 	}
 
 	// not really necessary but a santity check for future proofing against code adjustments that don't have full test coverage
 	// recognizing that just because it has a .json extension doesn't mean its a json file :(
 	// should never happen in production
-	errutil.MustConditionf(!isEntityDefinitionFile || path.Ext(details.Path) == ".json", "entity definition file does not have .json extension: %q", details.Path)
+	errutil.MustConditionf(!isEntityDefinitionFile || path.Ext(details.Path) == ".json", "entity definition file does not have .json extension: %v", logging.QuoteText(details.Path))
 
 	item := &MetadataEntityFile{
 		Entity: MetadataEntity{
@@ -427,7 +427,7 @@ func parseEntityPath(originalEntityPath string) (*entityPathDetails, error) {
 	relativeEntityPath := path.Join(relativePathSegments...)
 	subType, ok := parseMetadataSubType(*metadataType, relativeEntityPath)
 	if !ok {
-		return nil, fmt.Errorf(metadataTypeEntityPathNotSupported, (*metadataType).Name(), originalEntityPath)
+		return nil, fmt.Errorf(metadataTypeEntityPathNotSupported, (*metadataType).Name(), logging.QuoteText(originalEntityPath))
 	}
 
 	details := &entityPathDetails{
@@ -451,15 +451,15 @@ func parseMetadataType(originalEntityPath string) (string, *MetadataType, []stri
 	normalizedEntityPath := filepath.ToSlash(filepath.Clean(originalEntityPath))
 	directory := path.Dir(normalizedEntityPath)
 	if path.IsAbs(directory) || directory == "" || directory == "." {
-		return "", nil, nil, fmt.Errorf("directory name matching a valid metadata type name must exist in entity path: %q", originalEntityPath)
+		return "", nil, nil, fmt.Errorf("directory name matching a valid metadata type name must exist in entity path: %v", logging.QuoteText(originalEntityPath))
 	}
 
 	// Find the top/root level folder
 	dirSplit := strings.Split(directory, "/")
 	metadataName, subFolders := dirSplit[0], dirSplit[1:]
 	metadataType := enum.Parse(MetadataTypes, MetadataTypeValue(metadataName))
 	if metadataType == nil {
-		return "", nil, nil, fmt.Errorf("invalid metadata type name %q for entity path: %q", metadataName, originalEntityPath)
+		return "", nil, nil, fmt.Errorf("invalid metadata type name %v for entity path: %v", logging.QuoteText(metadataName), logging.QuoteText(originalEntityPath))
 	}
 
 	return normalizedEntityPath, metadataType, subFolders, nil

pkg/metadata/metadata_test.go

@@ -11,6 +11,7 @@ import (
 
 	"github.com/bobg/go-generics/v4/set"
 	"github.com/orsinium-labs/enum"
+	"github.com/skuid/skuid-cli/pkg/logging"
 	"github.com/skuid/skuid-cli/pkg/metadata"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -294,7 +295,7 @@ func (suite *NlxMetadataTestSuite) TestGetFieldValue() {
 		{
 			description:    "bad",
 			given:          metadata.MetadataType{"bad"},
-			wantPanicError: fmt.Errorf("unable to locate metadata field for metadata type %q", "bad"),
+			wantPanicError: fmt.Errorf("unable to locate metadata field for metadata type %v", "bad"),
 		},
 	} {
 		suite.Run(tc.description, func() {
@@ -363,7 +364,7 @@ func (suite *NlxMetadataTestSuite) TestFilterItem() {
 					Type: metadata.MetadataType{"bad"},
 				},
 			},
-			wantPanicError: fmt.Errorf("unable to locate metadata field for metadata type %q", "bad"),
+			wantPanicError: fmt.Errorf("unable to locate metadata field for metadata type %v", "bad"),
 		},
 	}
 
@@ -1273,7 +1274,7 @@ func TestNewMetadataEntityFile(t *testing.T) {
 			if tc.wantError != nil {
 				assert.EqualError(t, err, tc.wantError.Error(), "expected not nil err, got nil for path %v", tc.givePath)
 			} else {
-				assert.NoError(t, err, "expected nil err, got not nil for path %q", tc.givePath)
+				assert.NoError(t, err, "expected nil err, got not nil for path %v", logging.QuoteText(tc.givePath))
 			}
 			assert.Equal(t, tc.wantEntityFile, entityFile)
 		})
@@ -1743,15 +1744,15 @@ func TestMetadataEntityTestSuite(t *testing.T) {
 }
 
 func createPathError(mdt metadata.MetadataType, path string) error {
-	return fmt.Errorf("metadata type %q does not support the entity path: %q", mdt.Name(), filepath.FromSlash(path))
+	return fmt.Errorf("metadata type %v does not support the entity path: %v", mdt.Name(), logging.QuoteText(filepath.FromSlash(path)))
 }
 
 func createContainMetadataNameError(path string) error {
-	return fmt.Errorf("directory name matching a valid metadata type name must exist in entity path: %q", filepath.FromSlash(path))
+	return fmt.Errorf("directory name matching a valid metadata type name must exist in entity path: %v", logging.QuoteText(filepath.FromSlash(path)))
 }
 
 func createInvalidMetadataNameError(typename string, path string) error {
-	return fmt.Errorf("invalid metadata type name %q for entity path: %q", typename, filepath.FromSlash(path))
+	return fmt.Errorf("invalid metadata type name %v for entity path: %v", logging.QuoteText(typename), logging.QuoteText(filepath.FromSlash(path)))
 }
 
 func createEntity(t *testing.T, entityPath string) metadata.MetadataEntity {

pkg/util/strings.go

@@ -8,6 +8,7 @@ import (
 	"github.com/bobg/go-generics/v4/slices"
 	"github.com/goschtalt/approx"
 	"github.com/itlightning/dateparse"
+	"github.com/skuid/skuid-cli/pkg/logging"
 )
 
 type timeFormat struct {
@@ -45,7 +46,7 @@ func StringSliceContainsAnyKey(strings []string, keys []string) bool {
 func GetTime(value string, reference time.Time, noFuture bool) (time.Time, error) {
 	value = strings.TrimSpace(value)
 	if value == "" || value == "0" {
-		return time.Time{}, fmt.Errorf("value is not a valid time or duration: %q", value)
+		return time.Time{}, fmt.Errorf("value is not a valid time or duration: %v", logging.QuoteText(value))
 	}
 
 	// try duration first for two reasons:
@@ -62,7 +63,7 @@ func GetTime(value string, reference time.Time, noFuture bool) (time.Time, error
 	}
 
 	if noFuture && t.Compare(reference) > 0 {
-		return time.Time{}, fmt.Errorf("value %q cannot represent a time in the future: %v", value, t.Format(time.RFC3339))
+		return time.Time{}, fmt.Errorf("value %v cannot represent a time in the future: %v", logging.QuoteText(value), logging.FormatTime(&t))
 	}
 
 	return t, nil
@@ -121,7 +122,7 @@ func parseTime(value string, reference time.Time) (time.Time, error) {
 		}
 	}
 
-	return time.Time{}, fmt.Errorf("failed to parse value as time or duration: %q", value)
+	return time.Time{}, fmt.Errorf("failed to parse value as time or duration: %v", logging.QuoteText(value))
 }
 
 func forceCurrentDate(t time.Time, reference time.Time) time.Time {

pkg/util/strings_test.go

@@ -66,7 +66,7 @@ func TestStringSliceContainsAnyKey(t *testing.T) {
 func TestGetTime(t *testing.T) {
 	locName := "America/Los_Angeles"
 	loc, err := time.LoadLocation(locName)
-	require.NoError(t, err, "unable to load timezone location information for %q", locName)
+	require.NoError(t, err, "unable to load timezone location information for %v", locName)
 	reference := time.Date(2006, 1, 2, 15, 4, 5, 0, loc)
 
 	// inSameClock returns a copy of t in the provided location. The copy

pkg/zip_test.go

@@ -14,6 +14,7 @@ import (
 	"github.com/bobg/go-generics/v4/slices"
 	"github.com/orsinium-labs/enum"
 	"github.com/skuid/skuid-cli/pkg"
+	"github.com/skuid/skuid-cli/pkg/logging"
 	"github.com/skuid/skuid-cli/pkg/metadata"
 	pkgmocks "github.com/skuid/skuid-cli/pkg/mocks"
 	"github.com/skuid/skuid-cli/pkg/util"
@@ -431,7 +432,7 @@ func (suite *ArchiveTestSuite) TestEntityValidationFailure() {
 				giveFS:             fsys,
 				giveFileUtil:       util.NewFileUtil(),
 				giveArchiveFilter:  nil,
-				wantError:          fmt.Errorf("one or more entities found in %#q were invalid", fmt.Sprint(fsys)),
+				wantError:          fmt.Errorf("one or more entities found in %v were invalid", logging.QuoteText(fmt.Sprint(fsys))),
 				wantResultFiles:    nil,
 				wantResultEntities: nil,
 			})