diff --git a/internal/app/comments.go b/internal/app/comments.go index 02834d68..2e37520e 100644 --- a/internal/app/comments.go +++ b/internal/app/comments.go @@ -337,11 +337,14 @@ $ tea %[1]s comment list %[2]s --json id,author,body`, words.group, words.exampl return err } } - items, err := (services.Comments{Client: t.client, Repository: t.Repo}).Page(cmd.Context(), n, limit, page) + result, err := (services.Comments{Client: t.client, Repository: t.Repo}).Page(cmd.Context(), n, limit, page) if err != nil { return err } - return a.writeComments(cmd, t, n, items) + if err := a.writeComments(cmd, t, n, result.Items); err != nil { + return err + } + return a.morePagesHint(cmd, result.More, page) }} pageFlags(list, &limit, &page) list.ValidArgsFunction = a.completeDiscussions(kind) diff --git a/internal/app/download.go b/internal/app/download.go index 5cd7c3cf..87da916b 100644 --- a/internal/app/download.go +++ b/internal/app/download.go @@ -172,6 +172,38 @@ func publish(cmd *cobra.Command, output string, private, replace bool, write fun if errors.Is(err, os.ErrExist) { return clierr.Wrap(clierr.Usage, fmt.Errorf("%s already exists", output)) } + if err != nil { + // Some filesystems, such as FAT and many network mounts, cannot hard link. + return copyExclusive(file.Name(), output, private, err) + } + return nil +} + +// copyExclusive copies the finished temporary file to output, which must not exist, when linking +// failed with linkErr. A failed copy removes the partial output. +func copyExclusive(temporary, output string, private bool, linkErr error) error { + mode := os.FileMode(0o666) + if private { + mode = 0o600 + } + destination, err := os.OpenFile(output, os.O_WRONLY|os.O_CREATE|os.O_EXCL, mode) + if errors.Is(err, os.ErrExist) { + return clierr.Wrap(clierr.Usage, fmt.Errorf("%s already exists", output)) + } + if err != nil { + return linkErr + } + source, err := os.Open(temporary) + if err == nil { + defer source.Close() + _, err = io.Copy(destination, source) + } + if closeErr := destination.Close(); err == nil { + err = closeErr + } + if err != nil { + _ = os.Remove(output) + } return err } diff --git a/internal/app/download_test.go b/internal/app/download_test.go index 37dcca21..76993349 100644 --- a/internal/app/download_test.go +++ b/internal/app/download_test.go @@ -184,3 +184,32 @@ func TestDownloadFlagsRegisterGhNames(t *testing.T) { require.Equal(t, "dir", flags.dir) require.True(t, cmd.Flags().Lookup("output-file").Hidden) } + +func TestDownloadCreatesTheMissingDirectory(t *testing.T) { + t.Chdir(t.TempDir()) + deps, _, _ := testDeps(t) + a := &application{deps: deps} + cmd := &cobra.Command{} + cmd.SetContext(context.Background()) + flags := downloadFlags{dir: filepath.Join("dist", "assets")} + path, _, err := flags.destination("asset.bin") + require.NoError(t, err) + require.NoError(t, a.download(cmd, flags, path, strings.NewReader("new"))) + data, err := os.ReadFile(filepath.Join("dist", "assets", "asset.bin")) + require.NoError(t, err) + require.Equal(t, "new", string(data)) +} + +func TestCopyExclusiveStandsInForFilesystemsWithoutHardLinks(t *testing.T) { + directory := t.TempDir() + temporary := filepath.Join(directory, ".tea-download-1") + require.NoError(t, os.WriteFile(temporary, []byte("body"), 0o600)) + output := filepath.Join(directory, "asset.bin") + linkErr := errors.New("operation not supported") + require.NoError(t, copyExclusive(temporary, output, true, linkErr)) + data, err := os.ReadFile(output) + require.NoError(t, err) + require.Equal(t, "body", string(data)) + require.ErrorContains(t, copyExclusive(temporary, output, true, linkErr), "already exists") + require.ErrorIs(t, copyExclusive(temporary, filepath.Join(directory, "missing", "asset.bin"), true, linkErr), linkErr) +} diff --git a/internal/app/ref.go b/internal/app/ref.go index 1d3506f9..a1c9719d 100644 --- a/internal/app/ref.go +++ b/internal/app/ref.go @@ -176,7 +176,10 @@ func (a *application) resolveRefs(cmd *cobra.Command, raws []string, kind refKin if selected.Repo != "" && !a.sameRepository(selected.Repo, parsed.Repo) { return target{}, nil, clierr.Wrap(clierr.Usage, fmt.Errorf("%s and %s are in different repositories; run one command per repository", source, raw)) } - selected, source = parsed, raw + // A reference without an instance must not replace one that names it, whatever the order. + if selected.Repo == "" || a.namesInstance(parsed.Repo) { + selected, source = parsed, raw + } } t, err := a.resolveRequest(cmd.Context(), targetRequest{requireRepo: true, authenticate: true, repo: selected.Repo, repoSource: source}) return t, numbers, err @@ -193,6 +196,17 @@ func (a *application) sameRepository(first, second string) bool { return oneErr == nil && twoErr == nil && strings.EqualFold(one.Slug(), two.Slug()) && (one.Host == "" || two.Host == "" || one.Host == two.Host) } +// namesInstance reports whether a repository argument names its instance, as a URL or +// HOST/OWNER/REPO does. +func (a *application) namesInstance(raw string) bool { + s, err := a.settings() + if err != nil { + return false + } + parsed, err := settings.ParseRepository(raw, s.Hosts.Hosts) + return err == nil && parsed.Host != "" +} + // pullSelector resolves the pull request a pr command addresses: N, #N, OWNER/REPO#N, a pull // request URL, a head branch, OWNER:BRANCH, or, without an argument, the pull request of the // checked-out branch. diff --git a/internal/app/ref_test.go b/internal/app/ref_test.go index ffafe458..9b721663 100644 --- a/internal/app/ref_test.go +++ b/internal/app/ref_test.go @@ -235,3 +235,20 @@ func TestExpandMeUsesTheLoginTeaActsAs(t *testing.T) { require.NoError(t, err) require.Equal(t, "bob", resolved.expandMe("@me")) } + +func TestResolveRefsKeepsTheReferenceThatNamesAnInstance(t *testing.T) { + a, serverURL := refApplication(t, nil) + // GITEA_TOKEN belongs to serverURL, so selecting the other instance is what fails here. + other := strings.Replace(serverURL, "127.0.0.1", "localhost", 1) + const mismatch = "matching the selected instance" + for _, raws := range [][]string{ + {other + "/bob/tool/issues/1", "bob/tool#2"}, + {"bob/tool#2", other + "/bob/tool/issues/1"}, + } { + _, _, err := a.resolveRefs(commandContext(), raws, issueRef) + require.ErrorContains(t, err, mismatch, raws) + } + a.options.repo = other + "/bob/tool" + _, _, err := a.resolveRef(commandContext(), "bob/tool#2", issueRef) + require.ErrorContains(t, err, mismatch, "an argument without an instance keeps the --repo one") +} diff --git a/internal/app/target.go b/internal/app/target.go index 7d01a613..b0a6ffad 100644 --- a/internal/app/target.go +++ b/internal/app/target.go @@ -209,6 +209,11 @@ func (a *application) repositorySource(ctx context.Context, t *target, s setting if err := a.agreesWithRepoFlag(req, s.Hosts.Hosts); err != nil { return "", err } + if a.options.repo != "" && !a.namesInstance(req.repo) && a.namesInstance(a.options.repo) { + // The argument agrees with --repo and only lacks its instance. + t.RepoSource = "--repo" + return a.options.repo, nil + } t.RepoSource = req.repoSource return req.repo, nil } diff --git a/internal/app/time_test.go b/internal/app/time_test.go index 20533c3a..f9f233f7 100644 --- a/internal/app/time_test.go +++ b/internal/app/time_test.go @@ -128,3 +128,13 @@ func TestTimeListTotalPipedCoversEmptyAndPagedListings(t *testing.T) { require.Equal(t, "1\talice/project#3\talice\t1m\t2026-10-01T10:00:00Z\nTotal: 1m\n", out.String()) require.Equal(t, "more results available; use --page 2\n", diagnostics.String()) } + +func TestCommentListHintsAtTheNextClientSidePage(t *testing.T) { + server := newAuthenticatedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(`[{"id":1,"body":"a","user":{"login":"alice"}},{"id":2,"body":"b","user":{"login":"alice"}}]`)) + })) + defer server.Close() + deps, _, diagnostics := serverDeps(t, server.URL) + require.NoError(t, Execute(context.Background(), deps, []string{"issue", "comment", "list", "3", "-R", "alice/project", "--limit", "1"})) + require.Equal(t, "more results available; use --page 2\n", diagnostics.String()) +} diff --git a/internal/auth/manager.go b/internal/auth/manager.go index f34cb1a1..76529fb8 100644 --- a/internal/auth/manager.go +++ b/internal/auth/manager.go @@ -87,6 +87,12 @@ func (m Manager) register(instance, user string, account settings.Account, optio } undo = append(undo, func() error { return m.credentials.Delete(instance, user, account.CredentialStore) }) } + if exists && credential != nil && !previous.Tokenless() { + // Logging in again replaces the credential; settings made with tea config set stay. + kept := previous + kept.AuthMethod, kept.CredentialStore = account.AuthMethod, account.CredentialStore + account = kept + } host.Users[user] = account host.User = user if options.OAuthClientID != "" { diff --git a/internal/auth/manager_test.go b/internal/auth/manager_test.go index 030e3900..81108604 100644 --- a/internal/auth/manager_test.go +++ b/internal/auth/manager_test.go @@ -237,3 +237,22 @@ func TestFailedReplacementRestoresTheCredential(t *testing.T) { _, err = credentials.Load("https://example.org", "alice", settings.StoreFile) require.Error(t, err) } + +func TestReplacingATokenKeepsTheAccountSSHSettings(t *testing.T) { + config := settings.New(t.TempDir()) + credentials := &Store{Dir: config.Dir, Keyring: &memoryKeyring{values: map[string]string{}}} + manager := newTestManager(t, config, credentials) + require.NoError(t, manager.Register("https://example.org", "alice", settings.StoreFile, settings.AuthToken, HostOptions{}, Credential{AccessToken: "old"})) + require.NoError(t, config.Transaction(settings.TargetHosts, func(s *settings.Snapshot) (func() error, error) { + host := s.Hosts.Hosts["https://example.org"] + account := host.Users["alice"] + account.SSHKey, account.SSHAgent = "/keys/id", true + host.Users["alice"] = account + s.Hosts.Hosts["https://example.org"] = host + return nil, nil + })) + require.NoError(t, manager.Register("https://example.org", "alice", settings.StoreKeyring, settings.AuthOAuth, HostOptions{Replace: true}, Credential{AccessToken: "new"})) + s, err := config.Load() + require.NoError(t, err) + require.Equal(t, settings.Account{AuthMethod: settings.AuthOAuth, CredentialStore: settings.StoreKeyring, SSHKey: "/keys/id", SSHAgent: true}, s.Hosts.Hosts["https://example.org"].Users["alice"]) +} diff --git a/internal/services/comments.go b/internal/services/comments.go index 0416548e..513669bd 100644 --- a/internal/services/comments.go +++ b/internal/services/comments.go @@ -53,12 +53,16 @@ func (s Comments) List(ctx context.Context, n int64) ([]Comment, error) { } // Page applies CLI pagination to the unpaginated comment collection. -func (s Comments) Page(ctx context.Context, n int64, limit, page int) ([]Comment, error) { +func (s Comments) Page(ctx context.Context, n int64, limit, page int) (Page[Comment], error) { items, err := s.List(ctx, n) if err != nil { - return nil, err + return Page[Comment]{}, err } - return pageOf(items, limit, page) + window, err := pageOf(items, limit, page) + if err != nil { + return Page[Comment]{}, err + } + return Page[Comment]{Items: window, More: len(items) > page*limit, Total: len(items), Counted: true}, nil } // NotPlainCommentError reports a comment that the single-comment endpoint answers with no body, diff --git a/internal/services/comments_test.go b/internal/services/comments_test.go index 07ee32a6..da139219 100644 --- a/internal/services/comments_test.go +++ b/internal/services/comments_test.go @@ -34,12 +34,17 @@ func TestCommentPaginationIsLocal(t *testing.T) { service := Comments{Client: client, Repository: "alice/project"} items, err := service.Page(context.Background(), 7, 2, 2) require.NoError(t, err) - require.Len(t, items, 1) - require.Equal(t, int64(3), items[0].ID) + require.Len(t, items.Items, 1) + require.Equal(t, int64(3), items.Items[0].ID) + require.False(t, items.More) + first, err := service.Page(context.Background(), 7, 2, 1) + require.NoError(t, err) + require.True(t, first.More) + require.Equal(t, 3, first.Total) items, err = service.Page(context.Background(), 7, 2, 3) require.NoError(t, err) - require.Empty(t, items) - require.NotNil(t, items) + require.Empty(t, items.Items) + require.NotNil(t, items.Items) item, err := service.Create(context.Background(), 7, "text") require.NoError(t, err) require.Equal(t, "alice", item.Author)