diff --git a/docs/migration-1.0.md b/docs/migration-1.0.md index d1f89548..43feeb3b 100644 --- a/docs/migration-1.0.md +++ b/docs/migration-1.0.md @@ -150,9 +150,10 @@ tea 0.16 cloned over SSH when the login had an SSH key; tea 1.0 follows | `--debug` / `--vvv` | `--debug` | | `--confirm` / `-y`, `--force` / `-f` on destructive commands | `--yes` / `-y` | -`-R` now means `--repo`: `tea issue list -R origin` fails with a usage error -(`did you mean --remote origin?`) instead of selecting the `origin` remote. Use -`--remote origin`. +`-R` now means `--repo`, which takes `NAME`, `OWNER/REPO`, `HOST/OWNER/REPO` or +a URL: `tea issue list -R origin` no longer selects the `origin` remote. Inside +a checkout with that remote tea fails and suggests `--remote origin`; elsewhere +`origin` is read as a repository of your account. Use `--remote origin`. New global flags: `--no-input` disables prompts, editors and browsers, `--yes` (`-y`) accepts confirmations, `--cwd` selects the directory used for repository diff --git a/internal/app/action_runs.go b/internal/app/action_runs.go index 2e233a54..ad891b9f 100644 --- a/internal/app/action_runs.go +++ b/internal/app/action_runs.go @@ -716,7 +716,7 @@ func (a *application) completeJobs() cobra.CompletionFunc { if err != nil { return nil, cobra.ShellCompDirectiveNoFileComp } - return a.apiCompletion(true, func(ctx context.Context, t target, _ string) ([]cobra.Completion, error) { + return a.apiCompletionIn(run.Repo, true, func(ctx context.Context, t target, _ string) ([]cobra.Completion, error) { jobs, err := services.Actions{Client: t.client, Repository: t.Repo}.Jobs(ctx, run.Number) return describe(jobs, func(job services.Job) (string, string) { return strconv.FormatInt(job.ID, 10), job.Name + " · " + cmp.Or(job.Conclusion, job.Status) diff --git a/internal/app/alias_completion_test.go b/internal/app/alias_completion_test.go index 78f8b39a..bab58520 100644 --- a/internal/app/alias_completion_test.go +++ b/internal/app/alias_completion_test.go @@ -116,6 +116,9 @@ func TestCompletionOffersPathsOnlyWherePathsAreExpected(t *testing.T) { "issue list --state ": cobra.ShellCompDirectiveNoFileComp, "issue create --body-file ": cobra.ShellCompDirectiveDefault, "issue create --attachment ": cobra.ShellCompDirectiveDefault, + "label export --output ": cobra.ShellCompDirectiveDefault, + "run download 5 --output ": cobra.ShellCompDirectiveDefault, + "config set ssh_key ": cobra.ShellCompDirectiveDefault, "api --input ": cobra.ShellCompDirectiveDefault, "auth login --ssh-key ": cobra.ShellCompDirectiveDefault, "ssh-key create ": cobra.ShellCompDirectiveDefault, @@ -157,14 +160,14 @@ func TestCompletionOffersPathsOnlyWherePathsAreExpected(t *testing.T) { func TestOfflineCompletions(t *testing.T) { deps, out, _ := testDeps(t) deps.Git = &fakeGit{results: map[string]string{ - `config --get-regexp ^remote\..*\.url$`: "remote.origin.url https://gitea.example.com/alice/project.git\nremote.my.fork.url git@gitea.example.com:bob/project.git\n", + `config --get-regexp ^remote\..*\.url$`: "remote.origin.url https://gitea.example.com/alice/project.git\nremote.my.fork.url git@gitea.example.com:bob/project.git\nremote.ci.url https://bot:s3cret@gitea.example.com/alice/project.git?token=x\n", "for-each-ref --format=%(refname:short) refs/heads": "main\nfeature/login\n", }} configureAccounts(t, deps, "https://gitea.example.com", "alice", "alice", "bob") for args, want := range map[string][]string{ "--host ": {"https://gitea.example.com\talice, bob"}, "--user ": {"alice\thttps://gitea.example.com (active)", "bob\thttps://gitea.example.com"}, - "--remote ": {"origin\thttps://gitea.example.com/alice/project.git", "my.fork\tgit@gitea.example.com:bob/project.git"}, + "--remote ": {"origin\thttps://gitea.example.com/alice/project.git", "my.fork\tgit@gitea.example.com:bob/project.git", "ci\thttps://gitea.example.com/alice/project.git"}, } { values, directive := complete(t, deps, out, strings.Split(args, " ")...) require.Equal(t, want, values, args) @@ -217,6 +220,68 @@ func TestRepositoryCompletionUsesRemotesAndInstance(t *testing.T) { require.Contains(t, tokens, "token alice-token") } +func TestLabelCompletionDiscoversTheRepository(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v1/repos/alice/project/labels": + _, _ = w.Write([]byte(`[{"name":"bug","description":"Broken"},{"name":"docs"}]`)) + case "/api/v1/users/alice": + _, _ = w.Write([]byte(`{"id":1,"login":"alice"}`)) + default: + w.WriteHeader(http.StatusNotFound) + } + })) + defer server.Close() + deps, out, _ := testDeps(t) + deps.Git = &fakeGit{results: map[string]string{ + "remote": "origin", "remote get-url origin": server.URL + "/alice/project.git", + }} + configureAccounts(t, deps, server.URL, "alice", "alice") + values, _ := complete(t, deps, out, "label", "delete", "") + require.Equal(t, []string{"bug\tBroken", "docs"}, values) +} + +func TestJobCompletionUsesTheRepositoryOfTheRunURL(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v1/repos/bob/tool/actions/runs/44/jobs": + if r.URL.Query().Get("page") == "1" { + _, _ = w.Write([]byte(`{"jobs":[{"id":9,"name":"Build","status":"success"}]}`)) + } else { + _, _ = w.Write([]byte(`{"jobs":[]}`)) + } + case "/api/v1/users/alice": + _, _ = w.Write([]byte(`{"id":1,"login":"alice"}`)) + default: + w.WriteHeader(http.StatusNotFound) + } + })) + defer server.Close() + deps, out, _ := testDeps(t) + deps.Git = &fakeGit{results: map[string]string{"remote": "origin", "remote get-url origin": server.URL + "/alice/project.git"}} + configureAccounts(t, deps, server.URL, "alice", "alice") + values, _ := complete(t, deps, out, "run", "logs", server.URL+"/bob/tool/actions/runs/44", "--job", "") + require.Equal(t, []string{"9\tBuild · success"}, values) +} + +func TestActionValueCompletionNeedsNoRepositoryForAnOrganization(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v1/orgs/acme/actions/variables": + _, _ = w.Write([]byte(`[{"name":"MODE","description":"Build mode"}]`)) + case "/api/v1/users/alice": + _, _ = w.Write([]byte(`{"id":1,"login":"alice"}`)) + default: + w.WriteHeader(http.StatusNotFound) + } + })) + defer server.Close() + deps, out, _ := testDeps(t) + configureAccounts(t, deps, server.URL, "alice", "alice") + values, _ := complete(t, deps, out, "variable", "delete", "--org", "acme", "") + require.Equal(t, []string{"MODE\tBuild mode"}, values) +} + func TestAPICompletionNeverRefreshesOrFails(t *testing.T) { requests := 0 server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { @@ -297,7 +362,8 @@ func TestInstanceCompletions(t *testing.T) { directive cobra.ShellCompDirective }{ {"pulls by prefix", a.completePulls("open"), nil, "1", []string{"13\tAdd login support", "17\tImprove greeting"}, cobra.ShellCompDirectiveNoFileComp}, - {"pulls skip given arguments and sanitize titles", a.completePulls("open"), []string{"13", "17"}, "", []string{"2\tFix]0;pwned docs"}, cobra.ShellCompDirectiveNoFileComp}, + {"pulls skip given arguments and sanitize titles", withoutGiven(a.completePulls("open")), []string{"13", "17"}, "", []string{"2\tFix]0;pwned docs"}, cobra.ShellCompDirectiveNoFileComp}, + {"flag values may equal a positional argument", a.completePulls("open"), []string{"13"}, "13", []string{"13\tAdd login support"}, cobra.ShellCompDirectiveNoFileComp}, {"labels are comma-separated", a.completeLabels(), nil, "bug,", []string{"bug,docs"}, cobra.ShellCompDirectiveNoSpace | cobra.ShellCompDirectiveNoFileComp}, {"labels with descriptions", a.completeLabels(), nil, "b", []string{"bug\tSomething is broken"}, cobra.ShellCompDirectiveNoSpace | cobra.ShellCompDirectiveNoFileComp}, {"users", a.completeUsers(), nil, "", []string{"alice\tAlice", "bob"}, cobra.ShellCompDirectiveNoSpace | cobra.ShellCompDirectiveNoFileComp}, @@ -331,7 +397,7 @@ func TestCompletionTargetAcceptsABareTokenHost(t *testing.T) { require.NoError(t, testManager(t, deps).Register(instance, "alice", "file", "token", auth.HostOptions{}, auth.Credential{AccessToken: "stored-token"})) deps.Getenv = envFunc(map[string]string{"TEA_HOST": "127.0.0.1:3999", "GITEA_TOKEN": "environment-token"}) a := &application{deps: deps} - resolved, err := a.completionTarget(context.Background(), false) + resolved, err := a.completionTarget(context.Background(), "", false) require.NoError(t, err) require.Equal(t, instance, resolved.Host) } diff --git a/internal/app/artifacts.go b/internal/app/artifacts.go index 82cd1a15..aaaf86bc 100644 --- a/internal/app/artifacts.go +++ b/internal/app/artifacts.go @@ -295,7 +295,7 @@ func (a *application) completeArtifactNames() cobra.CompletionFunc { if err != nil { return nil, cobra.ShellCompDirectiveNoFileComp } - return a.apiCompletion(true, func(ctx context.Context, t target, _ string) ([]cobra.Completion, error) { + return a.apiCompletionIn(run.Repo, true, func(ctx context.Context, t target, _ string) ([]cobra.Completion, error) { artifacts, err := services.Actions{Client: t.client, Repository: t.Repo}.AllArtifacts(ctx, run.Number) return describe(artifacts, func(artifact services.Artifact) (string, string) { return artifact.Name, terminal.Bytes(artifact.Size) }), err })(cmd, args, toComplete) diff --git a/internal/app/completion.go b/internal/app/completion.go index 98ea4cfd..9bfdad10 100644 --- a/internal/app/completion.go +++ b/internal/app/completion.go @@ -7,6 +7,7 @@ import ( "context" "errors" "maps" + "net/url" "slices" "strconv" "strings" @@ -228,7 +229,21 @@ func (a *application) gitRemotes(ctx context.Context) [][2]string { func (a *application) completeRemotes(cmd *cobra.Command, _ []string, toComplete string) ([]cobra.Completion, cobra.ShellCompDirective) { ctx, cancel := completionContext(cmd) defer cancel() - return matching(describe(a.gitRemotes(ctx), func(remote [2]string) (string, string) { return remote[0], remote[1] }), toComplete), cobra.ShellCompDirectiveNoFileComp + return matching(describe(a.gitRemotes(ctx), func(remote [2]string) (string, string) { return remote[0], withoutCredentials(remote[1]) }), toComplete), cobra.ShellCompDirectiveNoFileComp +} + +// withoutCredentials drops the user info and query of a remote URL, which can embed a password or +// token that the shell would print in its menu. An unparsable URL shows nothing. +func withoutCredentials(raw string) string { + if !strings.Contains(raw, "://") { + return raw // scp-like addresses carry no password. + } + u, err := url.Parse(raw) + if err != nil { + return "" + } + u.User, u.RawQuery, u.Fragment = nil, "", "" + return u.String() } // completeLocalBranches completes the branches of the local checkout. @@ -264,18 +279,25 @@ func (a *application) completeConfig(cmd *cobra.Command, args []string, toComple return a.completeHosts(cmd, args, toComplete) case args[0] == "remote": return a.completeRemotes(cmd, args, toComplete) + case args[0] == "ssh_key": + return completeFiles(cmd, args, toComplete) } return matching(configValues[args[0]], toComplete), cobra.ShellCompDirectiveNoFileComp } -// apiCompletion completes the values that list fetches from the selected instance, except those -// already given. It never prompts, refreshes or retries, and failures complete nothing rather than -// file names. +// apiCompletion completes the values that list fetches from the selected instance. It never +// prompts, refreshes or retries, and failures complete nothing rather than file names. func (a *application) apiCompletion(requireRepo bool, list func(ctx context.Context, t target, toComplete string) ([]cobra.Completion, error)) cobra.CompletionFunc { + return a.apiCompletionIn("", requireRepo, list) +} + +// apiCompletionIn is apiCompletion for the repository that a command argument names, such as the +// one of a run URL; an empty repo selects the repository like any other command. +func (a *application) apiCompletionIn(repo string, requireRepo bool, list func(ctx context.Context, t target, toComplete string) ([]cobra.Completion, error)) cobra.CompletionFunc { return func(cmd *cobra.Command, args []string, toComplete string) ([]cobra.Completion, cobra.ShellCompDirective) { ctx, cancel := completionContext(cmd) defer cancel() - t, err := a.completionTarget(ctx, requireRepo) + t, err := a.completionTarget(ctx, repo, requireRepo) var values []cobra.Completion if err == nil { values, err = list(ctx, t, toComplete) @@ -284,18 +306,14 @@ func (a *application) apiCompletion(requireRepo bool, list func(ctx context.Cont cobra.CompDebugln("tea: "+err.Error(), false) return nil, cobra.ShellCompDirectiveNoFileComp } - values = slices.DeleteFunc(values, func(value cobra.Completion) bool { - name, _, _ := strings.Cut(value, "\t") - return slices.Contains(args, name) - }) return matching(values, toComplete), cobra.ShellCompDirectiveNoFileComp } } // completionTarget resolves an API completion's target like resolve, but never prompts or rotates // credentials: an expired OAuth token or an encrypted SSH key completes nothing. -func (a *application) completionTarget(ctx context.Context, requireRepo bool) (target, error) { - t, err := a.resolveTarget(ctx, requireRepo, false) +func (a *application) completionTarget(ctx context.Context, repo string, requireRepo bool) (target, error) { + t, err := a.resolveRequest(ctx, targetRequest{requireRepo: requireRepo, repo: repo, repoSource: "argument"}) if err != nil { return t, err } @@ -515,19 +533,27 @@ func (a *application) completeVariables(scope func() services.ActionScope) cobra // actionScope applies the target's repository to a scope without organization or account. func actionScope(scope func() services.ActionScope, t target) services.ActionScope { selected := scope() - if selected.Organization == "" && !selected.Account { + if inRepository(selected) { selected.Repository = t.Repo } return selected } +// inRepository reports whether the scope is the repository's, which discovery must then select. +func inRepository(scope services.ActionScope) bool { + return scope.Organization == "" && !scope.Account +} + // pathFlags take local file paths, like every flag whose name ends in -file. -var pathFlags = []string{"asset", "attachment", "file", "input", "ssh-key"} +var pathFlags = []string{"asset", "attachment", "file", "input", "output", "ssh-key"} // completeDefaults completes paths only where a command takes them: path flags and flags marked // with MarkFlagFilename. Everything else without a completer completes nothing, by the root's // default directive, rather than file names. func completeDefaults(cmd *cobra.Command) { + if cmd.ValidArgsFunction != nil { + cmd.ValidArgsFunction = withoutGiven(cmd.ValidArgsFunction) + } for _, flags := range []*pflag.FlagSet{cmd.Flags(), cmd.PersistentFlags()} { flags.VisitAll(func(flag *pflag.Flag) { if _, registered := cmd.GetFlagCompletionFunc(flag.Name); registered { @@ -546,6 +572,19 @@ func completeDefaults(cmd *cobra.Command) { } } +// withoutGiven drops the positional completions that the arguments already name. Flag values are not +// filtered: Cobra passes the positional arguments to their completers as well, and a job ID may equal +// its run ID. +func withoutGiven(complete cobra.CompletionFunc) cobra.CompletionFunc { + return func(cmd *cobra.Command, args []string, toComplete string) ([]cobra.Completion, cobra.ShellCompDirective) { + values, directive := complete(cmd, args, toComplete) + return slices.DeleteFunc(values, func(value cobra.Completion) bool { + name, _, _ := strings.Cut(value, "\t") + return slices.Contains(args, name) + }), directive + } +} + // completeFiles leaves the completion of file paths to the shell. func completeFiles(*cobra.Command, []string, string) ([]cobra.Completion, cobra.ShellCompDirective) { return nil, cobra.ShellCompDirectiveDefault diff --git a/internal/app/labels.go b/internal/app/labels.go index e77509e9..16ce1f33 100644 --- a/internal/app/labels.go +++ b/internal/app/labels.go @@ -351,11 +351,10 @@ func (a *application) importLabels(cmd *cobra.Command, scope labelScope, file st // completeScopedLabels completes the label names of the repository or, with --org, of the // organization that *org names. func (a *application) completeScopedLabels(org *string) cobra.CompletionFunc { - return a.apiCompletion(false, func(ctx context.Context, t target, _ string) ([]cobra.Completion, error) { - if *org == "" && t.Repo == "" { - return nil, errRepositoryRequired - } - items, err := (services.Labels{Client: t.client, Repository: t.Repo, Organization: *org}).List(ctx, completionLimit, 1) - return describe(items, func(item services.Label) (string, string) { return item.Name, item.Description }), err - }) + return func(cmd *cobra.Command, args []string, toComplete string) ([]cobra.Completion, cobra.ShellCompDirective) { + return a.apiCompletion(*org == "", func(ctx context.Context, t target, _ string) ([]cobra.Completion, error) { + items, err := (services.Labels{Client: t.client, Repository: t.Repo, Organization: *org}).List(ctx, completionLimit, 1) + return describe(items, func(item services.Label) (string, string) { return item.Name, item.Description }), err + })(cmd, args, toComplete) + } } diff --git a/internal/app/ref_test.go b/internal/app/ref_test.go index 27ab1c14..ef70dc93 100644 --- a/internal/app/ref_test.go +++ b/internal/app/ref_test.go @@ -210,6 +210,10 @@ func TestResolveRefSelectsTheReferencedRepository(t *testing.T) { _, _, err = a.resolveRef(commandContext(), serverURL+"/bob/tool/issues/1", issueRef) require.Equal(t, clierr.Usage, clierr.ExitCode(err)) require.EqualError(t, err, serverURL+"/bob/tool/issues/1 points to bob/tool but --repo is bob/other") + a.options.repo = "not a repo" + _, _, err = a.resolveRef(commandContext(), serverURL+"/bob/tool/issues/1", issueRef) + require.Equal(t, clierr.Usage, clierr.ExitCode(err)) + require.ErrorContains(t, err, `invalid repository "not a repo"`) } func TestExpandMeUsesTheLoginTeaActsAs(t *testing.T) { diff --git a/internal/app/target.go b/internal/app/target.go index be4a49cc..f368eb28 100644 --- a/internal/app/target.go +++ b/internal/app/target.go @@ -302,8 +302,11 @@ func (a *application) agreesWithRepoFlag(req targetRequest, hosts map[string]set } selected, selectedErr := settings.ParseRepository(req.repo, hosts) explicit, explicitErr := settings.ParseRepository(a.options.repo, hosts) - if selectedErr != nil || explicitErr != nil { - return nil // Resolution reports the invalid one. + if explicitErr != nil { + return clierr.Wrap(clierr.Usage, explicitErr) // The argument's repository replaces --repo, so nothing else parses it. + } + if selectedErr != nil { + return nil // Resolution reports the invalid argument. } sameHost := selected.Host == "" || explicit.Host == "" || selected.Host == explicit.Host sameRepo := strings.EqualFold(selected.Name, explicit.Name) && (explicit.Owner == "" || strings.EqualFold(selected.Owner, explicit.Owner)) diff --git a/internal/settings/remote.go b/internal/settings/remote.go index 3c02a2d8..05f400b0 100644 --- a/internal/settings/remote.go +++ b/internal/settings/remote.go @@ -178,12 +178,21 @@ func ParseRepository(raw string, hosts map[string]Host) (RepositoryRef, error) { } // The longest configured instance prefix wins, so example.org/gitea/owner/name selects the // instance installed below /gitea rather than example.org. - best := "" + best, tied := "", false for instance := range hosts { prefix := bareInstance(instance) - if len(prefix) > len(bareInstance(best)) && strings.HasPrefix(strings.ToLower(trimmed), strings.ToLower(prefix)+"/") && ValidSlug(trimmed[len(prefix)+1:]) { - best = instance + if !strings.HasPrefix(strings.ToLower(trimmed), strings.ToLower(prefix)+"/") || !ValidSlug(trimmed[len(prefix)+1:]) { + continue } + switch longest := len(bareInstance(best)); { + case len(prefix) > longest: + best, tied = instance, false + case len(prefix) == longest && best != "": + tied = true // http and https of one host: the scheme decides, not map order. + } + } + if tied { + return RepositoryRef{}, fmt.Errorf("instance %q is configured for both http and https; include the scheme", bareInstance(best)) } if best != "" { owner, name, _ := strings.Cut(trimmed[len(bareInstance(best))+1:], "/") diff --git a/internal/settings/remote_test.go b/internal/settings/remote_test.go index 56b7b826..351671b8 100644 --- a/internal/settings/remote_test.go +++ b/internal/settings/remote_test.go @@ -102,6 +102,17 @@ func TestParseRepositoryAcceptsShorthands(t *testing.T) { require.Equal(t, "alice/project", RepositoryRef{Owner: "alice", Name: "project"}.Slug()) } +func TestParseRepositoryRejectsAnInstanceConfiguredForBothSchemes(t *testing.T) { + hosts := map[string]Host{"http://localhost:3001": {}, "https://localhost:3001": {}} + for range 20 { // Map iteration order is random. + _, err := ParseRepository("localhost:3001/alice/project", hosts) + require.ErrorContains(t, err, "include the scheme") + } + got, err := ParseRepository("https://localhost:3001/alice/project", hosts) + require.NoError(t, err) + require.Equal(t, "https://localhost:3001", got.Host) +} + func TestResolveHostAcceptsBareHosts(t *testing.T) { hosts := map[string]Host{"http://localhost:3001": {}, "https://example.org/gitea": {}} for raw, want := range map[string]string{