diff --git a/.go-version b/.go-version index bae5c7f6..20a1265c 100644 --- a/.go-version +++ b/.go-version @@ -1 +1 @@ -1.21.3 +1.21.4 diff --git a/internal/cli/cli_test.go b/internal/cli/cli_test.go index f55b1bf0..5cc9e655 100644 --- a/internal/cli/cli_test.go +++ b/internal/cli/cli_test.go @@ -48,8 +48,8 @@ const ( exitcodeMakesChanges = 91 exitcodeNoChanges = 90 exitcodeError = 92 - - testPlanCmdString = "plan --exit-code-no-changes=90 --exit-code-makes-changes=91 --exit-code-error=92" + strFailedToFindPack = "Failed to find pack" + testPlanCmdString = "plan --exit-code-no-changes=90 --exit-code-makes-changes=91 --exit-code-error=92" ) func TestCLI_CreateTestRegistry(t *testing.T) { @@ -102,7 +102,7 @@ func TestCLI_JobRunConflictingDeployment(t *testing.T) { result := runTestPackCmd(t, s, []string{"run", getTestPackPath(t, testPack), "--name=with-name"}) must.Eq(t, 1, result.exitCode) must.Eq(t, result.cmdErr.String(), "", must.Sprintf("cmdErr should be empty, but was %q", result.cmdErr.String())) - must.StrContains(t, result.cmdOut.String(), job.ErrExistsInDeployment{JobID: testPack, Deployment: testPack}.Error()) + must.StrContains(t, result.cmdOut.String(), helper.FirstRuneToUpper(job.ErrExistsInDeployment{JobID: testPack, Deployment: testPack}.Error())) // Confirm that it's still possible to update the existing pack expectGoodPackDeploy(t, runTestPackCmd(t, s, []string{"run", getTestPackPath(t, testPack)})) @@ -121,7 +121,7 @@ func TestCLI_JobRunConflictingNonPackJob(t *testing.T) { must.Eq(t, 1, result.exitCode) must.Eq(t, result.cmdErr.String(), "", must.Sprintf("cmdErr should be empty, but was %q", result.cmdErr.String())) - must.StrContains(t, result.cmdOut.String(), job.ErrExistsNonPack{JobID: testPack}.Error()) + must.StrContains(t, result.cmdOut.String(), helper.FirstRuneToUpper(job.ErrExistsNonPack{JobID: testPack}.Error())) }) } @@ -136,7 +136,7 @@ func TestCLI_JobRunConflictingJobWithMeta(t *testing.T) { result := runTestPackCmd(t, s, []string{"run", getTestPackPath(t, testPack)}) must.Eq(t, 1, result.exitCode) must.Eq(t, result.cmdErr.String(), "", must.Sprintf("cmdErr should be empty, but was %q", result.cmdErr.String())) - must.StrContains(t, result.cmdOut.String(), job.ErrExistsNonPack{JobID: testPack}.Error()) + must.StrContains(t, result.cmdOut.String(), helper.FirstRuneToUpper(job.ErrExistsNonPack{JobID: testPack}.Error())) }) } @@ -147,7 +147,7 @@ func TestCLI_JobRunFails(t *testing.T) { must.Eq(t, 1, result.exitCode) must.Eq(t, "", result.cmdErr.String(), must.Sprintf("cmdErr should be empty, but was %q", result.cmdErr.String())) - must.StrContains(t, result.cmdOut.String(), "Failed To Find Pack") + must.StrContains(t, result.cmdOut.String(), strFailedToFindPack) } func TestCLI_JobPlan(t *testing.T) { @@ -161,7 +161,7 @@ func TestCLI_JobPlan_BadJob(t *testing.T) { result := runTestPackCmd(t, s, []string{"plan", "fake-job"}) must.Eq(t, "", result.cmdErr.String(), must.Sprintf("cmdErr should be empty, but was %q", result.cmdErr.String())) - must.StrContains(t, result.cmdOut.String(), "Failed To Find Pack") + must.StrContains(t, result.cmdOut.String(), strFailedToFindPack) must.Eq(t, 255, result.exitCode) // Should return 255 indicating an error }) } @@ -177,7 +177,7 @@ func TestCLI_JobPlan_ConflictingDeployment(t *testing.T) { result := runTestPackCmd(t, s, []string{"run", testPack, testRegFlag, testRefFlag}) must.Eq(t, "", result.cmdErr.String(), must.Sprintf("cmdErr should be empty, but was %q", result.cmdErr.String())) - must.StrContains(t, result.cmdOut.String(), job.ErrExistsInDeployment{JobID: testPack, Deployment: testPack + "@latest"}.Error()) + must.StrContains(t, result.cmdOut.String(), helper.FirstRuneToUpper(job.ErrExistsInDeployment{JobID: testPack, Deployment: testPack + "@latest"}.Error())) must.Eq(t, 1, result.exitCode) }) } @@ -192,7 +192,7 @@ func TestCLI_JobPlan_ConflictingNonPackJob(t *testing.T) { // Now try to register the pack result := runTestPackCmd(t, s, []string{"plan", getTestPackPath(t, testPack)}) must.Eq(t, "", result.cmdErr.String(), must.Sprintf("cmdErr should be empty, but was %q", result.cmdErr.String())) - must.StrContains(t, result.cmdOut.String(), job.ErrExistsNonPack{JobID: testPack}.Error()) + must.StrContains(t, result.cmdOut.String(), helper.FirstRuneToUpper(job.ErrExistsNonPack{JobID: testPack}.Error())) must.Eq(t, 255, result.exitCode) // Should return 255 indicating an error }) } @@ -223,7 +223,7 @@ func TestCLI_PackPlan_OverrideExitCodes(t *testing.T) { // Now try to register the pack, should be error result := runTestPackCmd(t, s, testPlanCommand(t)) must.Eq(t, "", result.cmdErr.String(), must.Sprintf("cmdErr should be empty, but was %q", result.cmdErr.String())) - must.StrContains(t, result.cmdOut.String(), job.ErrExistsNonPack{JobID: testPack}.Error()) + must.StrContains(t, result.cmdOut.String(), helper.FirstRuneToUpper(job.ErrExistsNonPack{JobID: testPack}.Error())) must.Eq(t, exitcodeError, result.exitCode) // Should exit-code-error }) diff --git a/internal/cli/cli_v1_test.go b/internal/cli/cli_v1_test.go index 9a4c5d25..2b0e7ee8 100644 --- a/internal/cli/cli_v1_test.go +++ b/internal/cli/cli_v1_test.go @@ -46,7 +46,7 @@ func TestCLI_V1_JobRunConflictingDeployment(t *testing.T) { result := runTestPackV1Cmd(t, s, []string{"run", getTestPackV1Path(t, testPack), "--name=with-name"}) must.Eq(t, 1, result.exitCode) expectNoStdErrOutput(t, result) - must.StrContains(t, result.cmdOut.String(), job.ErrExistsInDeployment{JobID: testPack, Deployment: testPack}.Error()) + must.StrContains(t, result.cmdOut.String(), helper.FirstRuneToUpper(job.ErrExistsInDeployment{JobID: testPack, Deployment: testPack}.Error())) // Confirm that it's still possible to update the existing pack expectGoodPackDeploy(t, runTestPackV1Cmd(t, s, []string{"run", getTestPackV1Path(t, testPack)})) @@ -65,7 +65,7 @@ func TestCLI_V1_JobRunConflictingNonPackJob(t *testing.T) { must.Eq(t, 1, result.exitCode) expectNoStdErrOutput(t, result) - must.StrContains(t, result.cmdOut.String(), job.ErrExistsNonPack{JobID: testPack}.Error()) + must.StrContains(t, result.cmdOut.String(), helper.FirstRuneToUpper(job.ErrExistsNonPack{JobID: testPack}.Error())) }) } @@ -80,7 +80,7 @@ func TestCLI_V1_JobRunConflictingJobWithMeta(t *testing.T) { result := runTestPackV1Cmd(t, s, []string{"run", getTestPackV1Path(t, testPack)}) must.Eq(t, 1, result.exitCode) expectNoStdErrOutput(t, result) - must.StrContains(t, result.cmdOut.String(), job.ErrExistsNonPack{JobID: testPack}.Error()) + must.StrContains(t, result.cmdOut.String(), helper.FirstRuneToUpper(job.ErrExistsNonPack{JobID: testPack}.Error())) }) } @@ -91,7 +91,7 @@ func TestCLI_V1_JobRunFails(t *testing.T) { must.Eq(t, 1, result.exitCode) expectNoStdErrOutput(t, result) - must.StrContains(t, result.cmdOut.String(), "Failed To Find Pack") + must.StrContains(t, result.cmdOut.String(), strFailedToFindPack) } func TestCLI_V1_JobPlan(t *testing.T) { @@ -106,7 +106,7 @@ func TestCLI_V1_JobPlan_BadJob(t *testing.T) { must.Eq(t, 255, result.exitCode) // Should return 255 indicating an error must.Eq(t, "", result.cmdErr.String(), must.Sprintf("cmdErr should be empty, but was %q", result.cmdErr.String())) - must.StrContains(t, result.cmdOut.String(), "Failed To Find Pack") + must.StrContains(t, result.cmdOut.String(), strFailedToFindPack) }) } @@ -122,7 +122,7 @@ func TestCLI_V1_JobPlan_ConflictingDeployment(t *testing.T) { result := runTestPackV1Cmd(t, s, []string{"run", testPack, testRegFlag, testRefFlag}) must.Eq(t, 1, result.exitCode) expectNoStdErrOutput(t, result) - must.StrContains(t, result.cmdOut.String(), job.ErrExistsInDeployment{JobID: testPack, Deployment: testPack + "@latest"}.Error()) + must.StrContains(t, result.cmdOut.String(), helper.FirstRuneToUpper(job.ErrExistsInDeployment{JobID: testPack, Deployment: testPack + "@latest"}.Error())) }) } @@ -137,7 +137,7 @@ func TestCLI_V1_JobPlan_ConflictingNonPackJob(t *testing.T) { result := runTestPackV1Cmd(t, s, []string{"plan", getTestPackV1Path(t, testPack)}) must.Eq(t, 255, result.exitCode) // Should return 255 indicating an error expectNoStdErrOutput(t, result) - must.StrContains(t, result.cmdOut.String(), job.ErrExistsNonPack{JobID: testPack}.Error()) + must.StrContains(t, result.cmdOut.String(), helper.FirstRuneToUpper(job.ErrExistsNonPack{JobID: testPack}.Error())) }) } @@ -166,7 +166,7 @@ func TestCLI_V1_PackPlan_OverrideExitCodes(t *testing.T) { // Now try to register the pack, should make error result := runTestPackV1Cmd(t, s, testPlanCommand(t)) must.Eq(t, "", result.cmdErr.String(), must.Sprintf("cmdErr should be empty, but was %q", result.cmdErr.String())) - must.StrContains(t, result.cmdOut.String(), job.ErrExistsNonPack{JobID: testPack}.Error()) + must.StrContains(t, result.cmdOut.String(), helper.FirstRuneToUpper(job.ErrExistsNonPack{JobID: testPack}.Error())) must.Eq(t, exitcodeError, result.exitCode) // Should exit-code-error }) diff --git a/internal/pkg/errors/template_errors.go b/internal/pkg/errors/template_errors.go index 332442d7..ed977518 100644 --- a/internal/pkg/errors/template_errors.go +++ b/internal/pkg/errors/template_errors.go @@ -76,10 +76,12 @@ func (p *PackTemplateError) pos() string { if p.Line == 0 { return "" } - out = fmt.Sprint(p.Line) - if p.StartChar != 0 { - out += fmt.Sprintf(",%d", p.StartChar) + out = fmt.Sprintf("Ln %v", p.Line) + start := p.StartChar + if p.StartChar == 0 { + p.StartChar = 1 } + out += fmt.Sprintf(", Col %d", start) return out } @@ -116,11 +118,13 @@ func (p *PackTemplateError) parseExecError(execErr template.ExecError) { } func (p *PackTemplateError) extractSource() { - hasElement := true - in := p.Err.Error() + var a, b string + found := true - for hasElement { - if b, a, found := strings.Cut(in, ": "); found { + in := p.Err.Error() // in is mutated to over time + + for found { + if b, a, found = strings.Cut(in, ": "); found { // the first element is "template" if b == "template" { in = a @@ -141,8 +145,9 @@ func (p *PackTemplateError) extractSource() { p.Filename = parts[0] in = a continue + } else { + found = false } - hasElement = false } } } @@ -154,6 +159,27 @@ func (p *PackTemplateError) enhance() { if p.isV2Error() { p.enhanceV2Error() } + if ok, ce := p.hasCallingError(); ok { + // In this case, the calling error makes more sense as the given error + // and the template runtime error is more of a detail--switch them around. + p.Details = p.Error() + p.Err = fmt.Errorf("%v", ce) + } +} + +func (p *PackTemplateError) hasCallingError() (bool, string) { + const errorCallingPrefix = "error calling " + for _, e := range p.Extra { + if strings.HasPrefix(e, errorCallingPrefix) { + return true, + fmt.Sprintf( + "%s`%s`", + errorCallingPrefix, + strings.TrimPrefix(e, errorCallingPrefix), + ) + } + } + return false, "" } func (p *PackTemplateError) isNPE() bool { diff --git a/internal/pkg/helper/ucformat.go b/internal/pkg/helper/ucformat.go new file mode 100644 index 00000000..14db763f --- /dev/null +++ b/internal/pkg/helper/ucformat.go @@ -0,0 +1,25 @@ +package helper + +import ( + "strings" + "unicode" + "unicode/utf8" +) + +// FirstRuneToUpper converts first rune to upper case if necessary. +func FirstRuneToUpper(str string) string { + if str == "" { + return str + } + + r, size := utf8.DecodeRuneInString(str) + + if !unicode.IsLower(r) { + return str + } + + buf := strings.Builder{} + buf.WriteRune(unicode.ToUpper(r)) + buf.WriteString(str[size:]) + return buf.String() +} diff --git a/internal/testui/main.go b/internal/testui/main.go index ad575042..780902f1 100644 --- a/internal/testui/main.go +++ b/internal/testui/main.go @@ -206,8 +206,8 @@ func (ui *nonInteractiveTestUI) Error(msg string) { // ErrorWithContext satisfies the ErrorWithContext function on the UI // interface. func (ui *nonInteractiveTestUI) ErrorWithContext(err error, sub string, ctx ...string) { - ui.Error(helper.Title(sub)) - ui.Error(" Error: " + err.Error()) + ui.Error(helper.FirstRuneToUpper(sub)) + ui.Error(" Error: " + helper.FirstRuneToUpper(err.Error())) // Selectively promote Details and Suggestion from the context. var extractItem = func(ctx []string, key string) ([]string, string, bool) { @@ -231,9 +231,9 @@ func (ui *nonInteractiveTestUI) ErrorWithContext(err error, sub string, ctx ...s case 1: // There is something odd going on if we don't get a 2 split // if we get 1, print the whole thing out. - ui.Error(" " + splits[0]) + ui.Error(" " + helper.FirstRuneToUpper(splits[0])) default: - ui.Error(" " + splits[0] + ": " + strings.Join(splits[1:], ": ")) + ui.Error(" " + splits[0] + ": " + helper.FirstRuneToUpper(strings.Join(splits[1:], ": "))) } } } diff --git a/terminal/glint.go b/terminal/glint.go index be3468b0..7e625c5b 100644 --- a/terminal/glint.go +++ b/terminal/glint.go @@ -10,6 +10,7 @@ import ( "fmt" "io" "os" + "regexp" "slices" "strings" "text/tabwriter" @@ -24,6 +25,8 @@ import ( "github.com/hashicorp/nomad-pack/internal/pkg/helper" ) +var rePlaceholder = regexp.MustCompile("^<<.+>>$") + type glintUI struct { ctx context.Context d *glint.Document @@ -362,15 +365,15 @@ func (ui *glintUI) ErrorWithContext(err error, sub string, ctx ...string) { // Title the error output in red with the subject. d.Append(glint.Layout( glint.Style( - glint.Text(fmt.Sprintf("! %s\n", helper.Title(sub))), - glint.Color("red"), + glint.Text(fmt.Sprintf("! %s\n", helper.FirstRuneToUpper(sub))), + glint.Bold(), glint.Color("red"), ), ).Row()) // Add the error string as well as the error type to the output. d.Append(glint.Layout( glint.Style(glint.Text(" Error: "), glint.Bold()), - glint.Text(err.Error()), + glint.Text(helper.FirstRuneToUpper(err.Error())), ).Row()) // Selectively promote Details and Suggestion from the context. @@ -396,12 +399,12 @@ func (ui *glintUI) ErrorWithContext(err error, sub string, ctx ...string) { // There is something odd going on if we don't get a 2 split // if we get 1, print the whole thing out. d.Append(glint.Layout( - glint.Style(glint.Text(" " + splits[0])), + glint.Style(glint.Text(" " + helper.FirstRuneToUpper(splits[0]))), ).Row()) default: d.Append(glint.Layout( glint.Style(glint.Text(" "+splits[0]+": "), glint.Bold()), - glint.Text(strings.Join(splits[1:], ": "))).Row()) + glint.Text(helper.FirstRuneToUpper(strings.Join(splits[1:], ": ")))).Row()) } } } @@ -419,14 +422,28 @@ func (ui *glintUI) ErrorWithContext(err error, sub string, ctx ...string) { // Iterate the addition context items and append these to the output. for _, additionCTX := range ctx { + k, v, _ := strings.Cut(additionCTX, ": ") d.Append(glint.Layout( - glint.Style(glint.Text(fmt.Sprintf(" - %s", additionCTX))), + glint.Text(" • "), + glint.Style( + glint.Text(fmt.Sprintf("%s: ", k)), + glint.Color("lightYellow"), + ), + formatPlaceholder(v), ).Row()) } // Add a new line d.Append(glint.Layout(glint.Text("")).Row()) } +func formatPlaceholder(v string) glint.Component { + if rePlaceholder.MatchString(v) { + v = strings.ReplaceAll(strings.ReplaceAll(v, "<<", "«"), ">>", "»") + return glint.Style(glint.Text(v), glint.Italic()) + } + return glint.Text(v) +} + // Header implements UI func (ui *glintUI) Header(msg string) { ui.Output(msg, WithHeaderStyle()) diff --git a/terminal/noninteractive.go b/terminal/noninteractive.go index 1120c078..12a385f3 100644 --- a/terminal/noninteractive.go +++ b/terminal/noninteractive.go @@ -213,8 +213,8 @@ func (ui *nonInteractiveUI) Error(msg string) { // ErrorWithContext satisfies the ErrorWithContext function on the UI // interface. func (ui *nonInteractiveUI) ErrorWithContext(err error, sub string, ctx ...string) { - ui.Error(helper.Title(sub)) - ui.Error(" Error: " + err.Error()) + ui.Error(helper.FirstRuneToUpper(sub)) + ui.Error(" Error: " + helper.FirstRuneToUpper(err.Error())) // Selectively promote Details and Suggestion from the context. var extractItem = func(ctx []string, key string) ([]string, string, bool) { @@ -240,7 +240,7 @@ func (ui *nonInteractiveUI) ErrorWithContext(err error, sub string, ctx ...strin wrapped := wordwrap.WrapString(key, 78) lines := strings.Split(wrapped, "\n") for _, l := range lines { - ui.Error(" " + l) + ui.Error(" " + helper.FirstRuneToUpper(l)) } return } @@ -248,7 +248,7 @@ func (ui *nonInteractiveUI) ErrorWithContext(err error, sub string, ctx ...strin lines := strings.Split(wrapped, "\n") for i, l := range lines { if i == 0 { - ui.Error(fmt.Sprintf(" %s: %s", key, l)) + ui.Error(fmt.Sprintf(" %s: %s", key, helper.FirstRuneToUpper(l))) continue }