From 88d6bfc470c0ccd27a0f5b3add2e1836e12c17f3 Mon Sep 17 00:00:00 2001 From: Nathan Rijksen Date: Wed, 4 Oct 2023 13:21:22 -0700 Subject: [PATCH] Conform rationalize behaviour to standard --- internal/errs/userfacing.go | 6 +- internal/locale/locales/en-us.yaml | 20 +++++++ internal/runbits/rationalize/types.go | 21 +++---- internal/runbits/runtime/rationalize.go | 4 +- internal/runners/hello/hello_example.go | 6 +- internal/runners/push/push.go | 67 +++++++--------------- internal/runners/push/rationalize.go | 76 +++++++++++++++++++++++++ pkg/cmdlets/checkout/rationalize.go | 4 +- 8 files changed, 134 insertions(+), 70 deletions(-) create mode 100644 internal/runners/push/rationalize.go diff --git a/internal/errs/userfacing.go b/internal/errs/userfacing.go index c493ca876c..e7a6282300 100644 --- a/internal/errs/userfacing.go +++ b/internal/errs/userfacing.go @@ -32,7 +32,7 @@ func (e *userFacingError) InputError() bool { return e.input } -func NewUserFacingError(message string, tips ...string) *userFacingError { +func NewUserFacing(message string, opts ...ErrOpt) *userFacingError { return WrapUserFacing(nil, message) } @@ -72,8 +72,8 @@ func SetTips(tips ...string) ErrOpt { } } -func SetInput(v bool) ErrOpt { +func SetInput() ErrOpt { return func(err *userFacingError) { - err.input = v + err.input = true } } diff --git a/internal/locale/locales/en-us.yaml b/internal/locale/locales/en-us.yaml index 6198f5c5ae..da98377165 100644 --- a/internal/locale/locales/en-us.yaml +++ b/internal/locale/locales/en-us.yaml @@ -1729,6 +1729,26 @@ package_info_request: err_push_outdated: other: | Your project has new changes available that need to be merged first. Please first run `[ACTIONABLE]state pull[/RESET]` to update your project. +err_tip_push_outdated: + other: Run `[ACTIONABLE]state pull[/RESET]`. +err_push_not_authenticated: + other: In order to update your project you need to be authenticated, please run '[ACTIONABLE]state auth[/RESET]' to authenticate. +err_push_headless: + other: No project found, you may need to create one first. +push_push_tip_headless_init: + other: Run [ACTIONABLE]state init[/RESET] to create a project with the State Tool. +push_push_tip_headless_cwd: + other: Navigate to a directory with an activestate.yaml. +err_push_nocommit: + other: You have nothing to push, make some changes first with [ACTIONABLE]state install[/RESET]. +err_push_create_nonunique: + other: The project name [NOTICE]{{.V0}}[/RESET] is already in use. +err_push_create_project_aborted: + other: Project creation aborted by user. +err_push_no_permission: + other: You do not have permission to push to {{.V0}}. +err_push_target_invalid_history: + other: The targets commit history does not match your local commit history. Are you pushing to the right project? err_pull_incompatible: other: | The remote project has an incompatible commit history. Target a different project with `[ACTIONABLE--set-project[/RESET]` or reset your local checkout with `[ACTIONABLE]state reset[/RESET]`. diff --git a/internal/runbits/rationalize/types.go b/internal/runbits/rationalize/types.go index 966c71ef1a..13d54f2464 100644 --- a/internal/runbits/rationalize/types.go +++ b/internal/runbits/rationalize/types.go @@ -1,22 +1,15 @@ package rationalize +import "errors" + // Inner is just an alias because otherwise external use of this struct would not be able to construct the error // property, and we want to keep the boilerplate minimal. type Inner error -type ErrNoProject struct { - Inner -} - -type ErrNotAuthenticated struct { - Inner -} +// ErrNoProject communicates that we were unable to find an activestate.yaml +var ErrNoProject = errors.New("no project") -type ErrActionAborted struct { - Inner -} +// ErrNotAuthenticated communicates that the user is not logged in +var ErrNotAuthenticated = errors.New("not authenticated") -type ErrPermission struct { - Inner - Details interface{} -} +var ErrActionAborted = errors.New("aborted by user") diff --git a/internal/runbits/runtime/rationalize.go b/internal/runbits/runtime/rationalize.go index 38ede373f7..b101338fa7 100644 --- a/internal/runbits/runtime/rationalize.go +++ b/internal/runbits/runtime/rationalize.go @@ -29,12 +29,12 @@ func rationalizeError(auth *authentication.Auth, proj *project.Project, rerr *er branches, err := model.BranchNamesForProjectFiltered(proj.Owner(), proj.Name(), proj.BranchName()) if err == nil && len(branches) > 1 { // Suggest alternate branches - *rerr = errs.NewUserFacingError(locale.Tr( + *rerr = errs.NewUserFacing(locale.Tr( "err_alternate_branches", errNoMatchingPlatform.HostPlatform, errNoMatchingPlatform.HostArch, proj.BranchName(), strings.Join(branches, "\n - "))) } else { - *rerr = errs.NewUserFacingError(locale.Tr( + *rerr = errs.NewUserFacing(locale.Tr( "err_no_platform_data_remains", errNoMatchingPlatform.HostPlatform, errNoMatchingPlatform.HostArch)) } diff --git a/internal/runners/hello/hello_example.go b/internal/runners/hello/hello_example.go index 7eb3d2e7d6..72d1d79507 100644 --- a/internal/runners/hello/hello_example.go +++ b/internal/runners/hello/hello_example.go @@ -8,6 +8,8 @@ package hello import ( + "errors" + "github.com/ActiveState/cli/internal/errs" "github.com/ActiveState/cli/internal/locale" "github.com/ActiveState/cli/internal/output" @@ -70,7 +72,7 @@ func rationalizeError(err *error) { // Ensure we wrap the top-level error returned from the runner and not // the unpacked error that we are inspecting. *err = errs.WrapUserFacing(*err, locale.Tl("hello_err_no_name", "Cannot say hello because no name was provided.")) - case errs.Matches(*err, &rationalize.ErrNoProject{}): + case errors.Is(*err, rationalize.ErrNoProject): // It's useful to offer users reasonable tips on recourses. *err = errs.WrapUserFacing( *err, @@ -89,7 +91,7 @@ func (h *Hello) Run(params *RunParams) (rerr error) { h.out.Print(locale.Tl("hello_notice", "This command is for example use only")) if h.project == nil { - return &rationalize.ErrNoProject{errs.New("Not in a project directory")} + return rationalize.ErrNoProject } // Reusable runner logic is contained within the runbits package. diff --git a/internal/runners/push/push.go b/internal/runners/push/push.go index 0891cfedca..6199678fc5 100644 --- a/internal/runners/push/push.go +++ b/internal/runners/push/push.go @@ -62,51 +62,24 @@ const ( intendCreateProject = 0x0008 ) -type errNoChanges struct{ *errs.WrapperError } -type errProjectInUse struct { - *errs.WrapperError +var errNoChanges = errors.New("no changes") + +var errTargetInvalidHistory = errors.New("local and remove histories do not match") + +type errProjectNameInUse struct { + error Namespace *project.Namespaced } -type errHistoryMismatch struct{ *errs.WrapperError } -type errFastForward struct{ *errs.WrapperError } - -func processError(err *error) { - if err == nil { - return - } - switch { - case errs.Matches(*err, &rationalize.ErrNotAuthenticated{}): - *err = errs.WrapUserFacing(*err, locale.Tl("err_push_not_authenticated", "In order to update your project you need to be authenticated, please run '[ACTIONABLE]state auth[/RESET]' to authenticate.")) - case errs.Matches(*err, &rationalize.ErrNoProject{}): - *err = errs.WrapUserFacing(*err, locale.Tl("err_push_headless", "You must first create a project."), - errs.SetTips( - locale.Tl("push_headless_push_tip_state_init", "Run [ACTIONABLE]state init[/RESET] to create a project with the State Tool."), - )) - case errs.Matches(*err, &errNoChanges{}): - *err = errs.WrapUserFacing(*err, locale.Tl("err_push_nocommit", "You have nothing to push, make some changes first with [ACTIONABLE]state install[/RESET].")) - case errs.Matches(*err, &errProjectInUse{}): - e := &errProjectInUse{} - errors.As(*err, &e) - *err = errs.WrapUserFacing(*err, locale.Tl( - "err_push_create_nonunique", - "The project [NOTICE]{{.V0}}[/RESET] is already in use.", e.Namespace.String())) - case errs.Matches(*err, &rationalize.ErrActionAborted{}): - *err = errs.WrapUserFacing(*err, locale.Tl("push_create_project_aborted", "Project creation aborted by user")) - case errs.Matches(*err, &rationalize.ErrPermission{}): - e := &rationalize.ErrPermission{} - errors.As(*err, &e) - *err = errs.WrapUserFacing(*err, locale.Tl("push_project_branch_no_permission", "You do not have permission to push to {{.V0}}.", e.Details.(*project.Namespaced).String())) - case errs.Matches(*err, &errHistoryMismatch{}): - *err = errs.WrapUserFacing(*err, locale.Tl("err_mergecommit_customtarget", "The targets commit history does not match your local commit history. Are you pushing to the right project?")) - case errs.Matches(*err, &errFastForward{}): - *err = errs.WrapUserFacing(*err, locale.T("err_push_outdated"), - errs.SetTips(locale.Tl("err_tip_push_outdated", "Run `[ACTIONABLE]state pull[/RESET]`"))) - } +type errNoPermission struct { + error + Namespace *project.Namespaced } +var errPullNeeded = errors.New("pull needed") + func (r *Push) Run(params PushParams) (rerr error) { - defer processError(&rerr) + defer rationalizeError(&rerr) if err := r.verifyInput(); err != nil { return errs.Wrap(err, "verifyInput failed") @@ -181,7 +154,7 @@ func (r *Push) Run(params PushParams) (rerr error) { var projectCreated bool if intend&intendCreateProject > 0 || targetPjm == nil { if targetPjm != nil { - return &errProjectInUse{errs.New("The project is already in use"), targetNamespace} + return &errProjectNameInUse{errs.New("project name in use"), targetNamespace} } // If the user didn't necessarily intend to create the project we should ask them for confirmation @@ -195,7 +168,7 @@ func (r *Push) Run(params PushParams) (rerr error) { return errs.Wrap(err, "Confirmation failed") } if !createProject { - return &rationalize.ErrActionAborted{errs.New("Aborted")} + return rationalize.ErrActionAborted } } @@ -243,13 +216,13 @@ func (r *Push) Run(params PushParams) (rerr error) { } if !errors.Is(err, model.ErrMergeFastForward) { if params.Namespace.IsValid() { - return &errHistoryMismatch{errs.New("Commit history does not match")} + return errTargetInvalidHistory } return errs.Wrap(err, "Could not detect if merge is necessary") } } if mergeStrategy != nil { - return &errFastForward{errs.New("Merge needed")} + return errPullNeeded } } @@ -257,7 +230,7 @@ func (r *Push) Run(params PushParams) (rerr error) { err = model.UpdateProjectBranchCommitWithModel(targetPjm, branch.Label, commitID) if err != nil { if errs.Matches(err, &model.ErrUpdateBranchAuth{}) { - return &rationalize.ErrPermission{errs.New("No permission to push"), targetNamespace} + return &errNoPermission{errors.New("no permission"), targetNamespace} } else { return errs.Wrap(err, "Failed to update new project %s to current commitID", targetNamespace.String()) } @@ -290,12 +263,12 @@ func (r *Push) Run(params PushParams) (rerr error) { func (r *Push) verifyInput() error { if !r.auth.Authenticated() { - return &rationalize.ErrNotAuthenticated{errs.New("Not authenticated")} + return rationalize.ErrNotAuthenticated } // Check if as.yaml exists if r.project == nil { - return &rationalize.ErrNoProject{errs.New("Not in project")} + return rationalize.ErrNoProject } commitID, err := localcommit.Get(r.project.Dir()) @@ -303,7 +276,7 @@ func (r *Push) verifyInput() error { return errs.Wrap(err, "Unable to get local commit") } if commitID == "" { - return &errNoChanges{errs.New("No changes")} + return errNoChanges } return nil diff --git a/internal/runners/push/rationalize.go b/internal/runners/push/rationalize.go new file mode 100644 index 0000000000..0d5995e8a1 --- /dev/null +++ b/internal/runners/push/rationalize.go @@ -0,0 +1,76 @@ +package push + +import ( + "errors" + + "github.com/ActiveState/cli/internal/errs" + "github.com/ActiveState/cli/internal/locale" + "github.com/ActiveState/cli/internal/runbits/rationalize" +) + +func rationalizeError(err *error) { + if err == nil { + return + } + + var projectNameInUseErr *errProjectNameInUse + var noPermissionErr *errNoPermission + + switch { + + // Not authenticated + case errors.Is(*err, rationalize.ErrNotAuthenticated): + *err = errs.WrapUserFacing(*err, + locale.T("err_push_not_authenticated"), + errs.SetInput()) + + // No activestate.yaml + case errors.Is(*err, rationalize.ErrNoProject): + *err = errs.WrapUserFacing(*err, + locale.T("err_push_headless"), + errs.SetInput(), + errs.SetTips( + locale.T("push_push_tip_headless_init"), + locale.T("push_push_tip_headless_cwd"), + )) + + // No changes were made + case errors.Is(*err, errNoChanges): + *err = errs.WrapUserFacing(*err, + locale.T("err_push_nocommit"), + errs.SetInput(), + ) + + // Project name is already in use + case errors.As(*err, &projectNameInUseErr): + *err = errs.WrapUserFacing(*err, + locale.Tr("err_push_create_nonunique", projectNameInUseErr.Namespace.String()), + errs.SetInput(), + ) + + // Project creation aborted + case errors.Is(*err, rationalize.ErrActionAborted): + *err = errs.WrapUserFacing(*err, + locale.T("err_push_create_project_aborted"), + errs.SetInput()) + + // No permission to push to project + case errors.As(*err, &noPermissionErr): + *err = errs.WrapUserFacing(*err, + locale.Tr("err_push_no_permission", noPermissionErr.Namespace.String()), + errs.SetInput()) + + // Custom target does not have a compatible history + case errors.Is(*err, errTargetInvalidHistory): + *err = errs.WrapUserFacing(*err, + locale.T("err_push_target_invalid_history"), + errs.SetInput()) + + // Need to pull first + case errors.Is(*err, errPullNeeded): + *err = errs.WrapUserFacing(*err, + locale.T("err_push_outdated"), + errs.SetInput(), + errs.SetTips(locale.T("err_tip_push_outdated"))) + } +} diff --git a/pkg/cmdlets/checkout/rationalize.go b/pkg/cmdlets/checkout/rationalize.go index c9dbc10bf1..6d546b5a14 100644 --- a/pkg/cmdlets/checkout/rationalize.go +++ b/pkg/cmdlets/checkout/rationalize.go @@ -18,13 +18,13 @@ func (c *Checkout) rationalizeError(err *error) { case errors.As(*err, &errAlreadyCheckedOut): *err = errs.WrapUserFacing( *err, locale.Tr("err_already_checked_out", errAlreadyCheckedOut.Path), - errs.SetInput(true), + errs.SetInput(), ) case errors.As(*err, &errProjectNotFound): *err = errs.WrapUserFacing(*err, locale.Tr("err_api_project_not_found", errProjectNotFound.Organization, errProjectNotFound.Project), errs.SetIf(!c.auth.Authenticated(), errs.SetTips(locale.T("tip_private_project_auth"))), - errs.SetInput(true), + errs.SetInput(), ) } }