diff --git a/internal/app/args.go b/internal/app/args.go index 0da1fbf2..1bc9a1ed 100644 --- a/internal/app/args.go +++ b/internal/app/args.go @@ -295,10 +295,11 @@ func argumentName(cmd *cobra.Command, argument string) string { if group == cmd { return text } - if noun := group.Name(); noun == "pr" { - return "pull request " + text + noun := group.Name() + if noun == "pr" { + noun = "pull request" } - return group.Name() + " " + text + return noun + " " + text } // localFlagsChanged reports whether any command-specific flag was supplied. diff --git a/internal/app/download.go b/internal/app/download.go index 9ed820c7..dbecb1ea 100644 --- a/internal/app/download.go +++ b/internal/app/download.go @@ -5,6 +5,7 @@ package app import ( "bufio" + "cmp" "errors" "fmt" "io" @@ -58,10 +59,10 @@ func (f downloadFlags) destination(name string) (path string, skip bool, err err } path = f.output if path == "" || isDirectory(path) { - if name != filepath.Base(name) || name == "." || name == ".." || strings.ContainsAny(name, `/\`) { + if name == "" || name == "." || name == ".." || strings.ContainsAny(name, `/\`) { return "", false, clierr.Wrap(clierr.Usage, fmt.Errorf("cannot use %q as a file name; pass --output", name)) } - path = filepath.Join(cmpOrDot(path, f.dir), name) + path = filepath.Join(cmp.Or(path, f.dir, "."), name) } if _, err := os.Lstat(path); err == nil { if f.skipExisting { @@ -76,15 +77,6 @@ func (f downloadFlags) destination(name string) (path string, skip bool, err err return path, false, nil } -func cmpOrDot(values ...string) string { - for _, value := range values { - if value != "" { - return value - } - } - return "." -} - func isDirectory(path string) bool { info, err := os.Stat(path) return (err == nil && info.IsDir()) || strings.HasSuffix(path, string(filepath.Separator)) || strings.HasSuffix(path, "/") diff --git a/internal/app/errors.go b/internal/app/errors.go index 197b1083..01ac67e1 100644 --- a/internal/app/errors.go +++ b/internal/app/errors.go @@ -10,6 +10,7 @@ import ( "fmt" "net" "net/http" + "net/url" "strings" "syscall" "time" @@ -206,7 +207,7 @@ func existsHint(ctx context.Context, apiErr *transport.APIError, conflict *servi var pull struct { URL string `json:"html_url"` } - if apiErr.Lookup(ctx, "repos/"+conflict.Scope+"/pulls/"+base+"/"+head, &pull) == http.StatusOK && pull.URL != "" { + if apiErr.Lookup(ctx, "repos/"+conflict.Scope+"/pulls/"+url.PathEscape(base)+"/"+url.PathEscape(head), &pull) == http.StatusOK && pull.URL != "" { return "see " + pull.URL } case "wiki page": diff --git a/internal/app/ref.go b/internal/app/ref.go index ef18437a..d713d16f 100644 --- a/internal/app/ref.go +++ b/internal/app/ref.go @@ -74,11 +74,14 @@ func parseRefURL(raw string, kind refKind) (ref, error) { return ref{}, invalid } segments := strings.Split(strings.Trim(u.Path, "/"), "/") - positive := func(i int) int64 { - if i >= len(segments) { - return 0 + at := func(i int) string { + if i < len(segments) { + return segments[i] } - n, err := strconv.ParseInt(segments[i], 10, 64) + return "" + } + positive := func(i int) int64 { + n, err := strconv.ParseInt(at(i), 10, 64) if err != nil || n < 1 { return 0 } @@ -88,12 +91,12 @@ func parseRefURL(raw string, kind refKind) (ref, error) { for i := 2; i < len(segments); i++ { result := ref{} switch { - case kind == runRef && segments[i] == "actions" && i+2 < len(segments) && segments[i+1] == "runs" && positive(i+2) > 0: + case kind == runRef && at(i) == "actions" && at(i+1) == "runs" && positive(i+2) > 0: result.Number = positive(i + 2) - if i+4 < len(segments) && segments[i+3] == "jobs" { + if at(i+3) == "jobs" { result.Job = positive(i + 4) } - case kind != runRef && (segments[i] == "issues" || segments[i] == "pulls") && positive(i+1) > 0: + case kind != runRef && (at(i) == "issues" || at(i) == "pulls") && positive(i+1) > 0: result.Number = positive(i + 1) default: continue @@ -188,6 +191,10 @@ func (a *application) sameRepository(first, second string) bool { // checked-out branch. func (a *application) pullSelector(cmd *cobra.Command, args []string) (target, int64, error) { if len(args) == 0 || args[0] == "" { + // The checked-out branch says nothing about a repository chosen with --repo. + if a.options.repo != "" { + return target{}, 0, clierr.Wrap(clierr.Usage, errors.New("pass a pull request number, URL or branch with --repo")) + } return a.currentPull(cmd.Context()) } raw := args[0] diff --git a/internal/app/ref_test.go b/internal/app/ref_test.go index 60352a3c..ad79b4c5 100644 --- a/internal/app/ref_test.go +++ b/internal/app/ref_test.go @@ -135,6 +135,11 @@ func TestPullSelectorDefaultsToTheCheckedOutBranch(t *testing.T) { require.Equal(t, tc.want, n) }) } + a, _ := refApplication(t, branch("feat/two", nil)) + a.options.repo = "alice/project" + _, _, err := a.pullSelector(commandContext(), nil) + require.Equal(t, clierr.Usage, clierr.ExitCode(err)) + require.EqualError(t, err, "pass a pull request number, URL or branch with --repo") } func TestPullSelectorAcceptsNumbersURLsAndBranches(t *testing.T) { diff --git a/internal/services/errors.go b/internal/services/errors.go index 73047ac6..b3778bd4 100644 --- a/internal/services/errors.go +++ b/internal/services/errors.go @@ -141,7 +141,7 @@ var refusals = []struct { statuses []int fragments []string }{ - {ReasonArchived, nil, []string{"is archived"}}, + {ReasonArchived, nil, []string{"repo is archived", "repository is archived", "> is archived"}}, {ReasonMerged, nil, []string{"already merged"}}, {ReasonDraft, []int{http.StatusMethodNotAllowed}, []string{"work in progress", "draft"}}, {ReasonOwnReview, nil, []string{"approve your own pull", "reject your own pull"}}, @@ -341,16 +341,24 @@ func instanceResource(parts []string) Resource { return Resource{Kind: "member", Key: strconv.Quote(at(3)), Scope: at(1)} case at(0) == "orgs" && at(1) != "" && at(2) == "repos": return Resource{Kind: "organization", Key: strconv.Quote(at(1)), Collection: "repository"} + case at(0) == "orgs" && at(1) != "" && at(2) == "teams": + return Resource{Kind: "organization", Key: strconv.Quote(at(1)), Collection: "team"} case at(0) == "orgs" && at(1) != "": return Resource{Kind: "organization", Key: strconv.Quote(at(1))} + case at(0) == "orgs": + return Resource{Collection: "organization"} case at(0) == "users" && at(1) != "": return Resource{Kind: "user", Key: strconv.Quote(at(1))} case at(0) == "admin" && at(1) == "users" && at(2) != "": return Resource{Kind: "user", Key: strconv.Quote(at(2))} + case at(0) == "admin" && at(1) == "users": + return Resource{Collection: "user"} case at(0) == "teams" && isNumber(at(1)): return Resource{Kind: "team", Key: at(1)} case at(0) == "user" && at(1) == "keys" && at(2) != "": return Resource{Kind: "SSH key", Key: at(2)} + case at(0) == "user" && at(1) == "keys": + return Resource{Collection: "SSH key"} case at(0) == "user" && at(1) == "repos": return Resource{Collection: "repository"} case at(0) == "notifications" && at(1) == "threads" && at(2) != "": diff --git a/internal/services/errors_test.go b/internal/services/errors_test.go index b63c59e0..97138934 100644 --- a/internal/services/errors_test.go +++ b/internal/services/errors_test.go @@ -51,6 +51,10 @@ func TestClassifyRequestNamesTheResource(t *testing.T) { {"wiki page exists", "POST", "/api/v1/repos/alice/x/wiki/new", "wiki page already exists [title: Home.md]", 400, `wiki page "Home" already exists in alice/x`}, {"repository exists", "POST", "/api/v1/user/repos", "The repository with the same name already exists.", 409, "repository already exists"}, {"org repository exists", "POST", "/api/v1/orgs/acme/repos", "The repository with the same name already exists.", 409, "repository already exists"}, + {"team exists", "POST", "/api/v1/orgs/acme/teams", "team already exists [org_id: 3, name: devs]", 422, "team already exists"}, + {"user exists", "POST", "/api/v1/admin/users", "user already exists [name: bob]", 422, "user already exists"}, + {"organization exists", "POST", "/api/v1/orgs", "user already exists [name: acme]", 422, "organization already exists"}, + {"archived label is no repository", "POST", "/api/v1/repos/alice/x/issues/1/labels", "label is archived", 422, "Gitea API returned HTTP 422 for POST /api/v1/repos/alice/x/issues/1/labels: label is archived"}, {"run completed", "POST", "/api/v1/repos/alice/x/actions/runs/35/cancel", "run is already completed", 409, "run 35 is already completed"}, {"run not done", "DELETE", "/api/v1/repos/alice/x/actions/runs/37", "this workflow run is not done", 400, "run 37 has not finished"}, {"workflow disabled", "POST", "/api/v1/repos/alice/x/actions/workflows/nightly.yaml/dispatches", "workflow is disabled", 403, "workflow nightly.yaml is disabled"},