fix: page hint for comment list, keep SSH settings on re-login, hard-link-free downloads, stable OWNER/REPO#N instance (follow gh)

Signed-off-by: bircni <bircni@icloud.com>
This commit is contained in:
bircni committed 2026-10-18 20:03:15 +02:00
1 parent dfb56eae67
commit 8305a89531
11 files changed
+153 -9

No files matched your search

+5 -2
View File
@@ -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)
+32
View File
@@ -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
}
+29
View File
@@ -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)
}
+14
View File
@@ -176,8 +176,11 @@ 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))
}
// 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.
+17
View File
@@ -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")
}
+5
View File
@@ -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
}
+10
View File
@@ -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())
}
+6
View File
@@ -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 != "" {
+19
View File
@@ -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"])
}
+7 -3
View File
@@ -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,
+9 -4
View File
@@ -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)