refactor: remove helpers the new output and prompts left unused

Drop writeDownload, terminal.Require, terminal.Confirm, terminal.Row,
terminal.Record and Pulls.FindOpenByHead, which no command calls anymore,
and point their tests at the code that replaced them.

Signed-off-by: bircni <bircni@icloud.com>
This commit is contained in:
bircni committed 2026-10-12 10:00:05 +02:00
1 parent eff6efe759
commit 50a604f169
8 files changed
+34 -118

No files matched your search

-14
View File
@@ -19,7 +19,6 @@ import (
"gitea.dev/tea/internal/clierr" "gitea.dev/tea/internal/clierr"
"gitea.dev/tea/internal/terminal" "gitea.dev/tea/internal/terminal"
"github.com/spf13/cobra" "github.com/spf13/cobra"
"golang.org/x/term"
) )
// downloadFlags are the destination flags of a download command. // downloadFlags are the destination flags of a download command.
@@ -95,19 +94,6 @@ func (a *application) download(cmd *cobra.Command, f downloadFlags, path string,
}) })
} }
func writeDownload(cmd *cobra.Command, body io.Reader, output string) error {
if output == "" || output == "-" {
return writeTerminalSafe(cmd.OutOrStdout(), isTerminal(cmd.OutOrStdout()), body)
}
return publish(cmd, output, false, false, func(destination io.Writer) error { _, err := io.Copy(destination, body); return err })
}
// isTerminal reports whether w is a terminal, for writers without injected stream capabilities.
func isTerminal(w io.Writer) bool {
file, ok := w.(*os.File)
return ok && term.IsTerminal(int(file.Fd()))
}
// binarySniffLength is how much of a download decides whether it is text. // binarySniffLength is how much of a download decides whether it is text.
const binarySniffLength = 8 << 10 const binarySniffLength = 8 << 10
+7 -4
View File
@@ -40,21 +40,24 @@ func TestDownloadsPublishOnlyCompleteFiles(t *testing.T) {
cmd.SetContext(context.Background()) cmd.SetContext(context.Background())
var stdout bytes.Buffer var stdout bytes.Buffer
cmd.SetOut(&stdout) cmd.SetOut(&stdout)
require.NoError(t, writeDownload(cmd, bytes.NewBufferString("archive"), "-")) copyBody := func(body io.Reader) func(io.Writer) error {
return func(destination io.Writer) error { _, err := io.Copy(destination, body); return err }
}
require.NoError(t, writeTerminalSafe(cmd.OutOrStdout(), false, bytes.NewBufferString("archive")))
require.Equal(t, "archive", stdout.String()) require.Equal(t, "archive", stdout.String())
directory := t.TempDir() directory := t.TempDir()
output := filepath.Join(directory, "logs.zip") output := filepath.Join(directory, "logs.zip")
require.ErrorIs(t, writeDownload(cmd, failingDownload{}, output), io.ErrUnexpectedEOF) require.ErrorIs(t, publish(cmd, output, false, false, copyBody(failingDownload{})), io.ErrUnexpectedEOF)
_, err := os.Stat(output) _, err := os.Stat(output)
require.True(t, errors.Is(err, os.ErrNotExist)) require.True(t, errors.Is(err, os.ErrNotExist))
entries, err := os.ReadDir(directory) entries, err := os.ReadDir(directory)
require.NoError(t, err) require.NoError(t, err)
require.Empty(t, entries) require.Empty(t, entries)
require.NoError(t, writeDownload(cmd, bytes.NewBufferString("archive"), output)) require.NoError(t, publish(cmd, output, false, false, copyBody(bytes.NewBufferString("archive"))))
info, err := os.Stat(output) info, err := os.Stat(output)
require.NoError(t, err) require.NoError(t, err)
require.Equal(t, umaskMode(t), info.Mode().Perm(), "downloads are not private files") require.Equal(t, umaskMode(t), info.Mode().Perm(), "downloads are not private files")
require.ErrorContains(t, writeDownload(cmd, bytes.NewBufferString("replacement"), output), "already exists") require.ErrorContains(t, publish(cmd, output, false, false, copyBody(bytes.NewBufferString("replacement"))), "already exists")
data, err := os.ReadFile(output) data, err := os.ReadFile(output)
require.NoError(t, err) require.NoError(t, err)
require.Equal(t, "archive", string(data)) require.Equal(t, "archive", string(data))
-26
View File
@@ -250,32 +250,6 @@ func (s Pulls) ListOpen(ctx context.Context) ([]Pull, error) {
}) })
} }
// FindOpenByHead returns the unique open pull whose head branch matches name.
func (s Pulls) FindOpenByHead(ctx context.Context, head string) (Pull, error) {
head = strings.TrimSpace(head)
if head == "" {
return Pull{}, fmt.Errorf("head branch cannot be empty")
}
items, err := s.ListOpen(ctx)
if err != nil {
return Pull{}, err
}
matches := []Pull{}
for _, item := range items {
if item.Head == head {
matches = append(matches, item)
}
}
switch len(matches) {
case 0:
return Pull{}, fmt.Errorf("no open pull request uses head branch %q", head)
case 1:
return matches[0], nil
default:
return Pull{}, fmt.Errorf("multiple open pull requests use head branch %q; supply an explicit number", head)
}
}
// Create opens a pull request. // Create opens a pull request.
func (s Pulls) Create(ctx context.Context, opts PullOptions) (Pull, error) { func (s Pulls) Create(ctx context.Context, opts PullOptions) (Pull, error) {
// TODO(sdk): CreatePullRequestOption lacks allow_maintainer_edit. // TODO(sdk): CreatePullRequestOption lacks allow_maintainer_edit.
+4 -5
View File
@@ -57,7 +57,7 @@ func TestPullMilestoneClearUsesIssueEndpoint(t *testing.T) {
} }
} }
func TestFindOpenByHeadPagesPastSmallServerLimitsAndMatchesExactly(t *testing.T) { func TestListOpenPagesPastSmallServerLimits(t *testing.T) {
client := newTestClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { client := newTestClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
assert.Equal(t, "/api/v1/repos/alice/project/pulls", r.URL.Path) assert.Equal(t, "/api/v1/repos/alice/project/pulls", r.URL.Path)
switch r.URL.Query().Get("page") { switch r.URL.Query().Get("page") {
@@ -70,11 +70,10 @@ func TestFindOpenByHeadPagesPastSmallServerLimitsAndMatchesExactly(t *testing.T)
} }
})) }))
service := Pulls{Client: client, Repository: "alice/project"} service := Pulls{Client: client, Repository: "alice/project"}
item, err := service.FindOpenByHead(context.Background(), "feature") items, err := service.ListOpen(context.Background())
require.NoError(t, err) require.NoError(t, err)
require.EqualValues(t, 2, item.Number) require.Len(t, items, 2)
_, err = service.FindOpenByHead(context.Background(), "FEATURE") require.Equal(t, "feature", items[1].Head)
require.ErrorContains(t, err, "no open pull request")
} }
func TestListOpenRejectsEndlessPagination(t *testing.T) { func TestListOpenRejectsEndlessPagination(t *testing.T) {
-11
View File
@@ -205,11 +205,6 @@ func RequireInput(ctx context.Context, value *string, flag, label string, valida
return true, nil return true, nil
} }
// Require is RequireInput titled by the flag name, for callers without a human label.
func Require(ctx context.Context, value *string, flag string, streams Streams, policy Policy, prompt Prompter) (bool, error) {
return RequireInput(ctx, value, flag, flag, nil, streams, policy, prompt)
}
// Confirmation describes the question asked before an action. // Confirmation describes the question asked before an action.
type Confirmation struct { type Confirmation struct {
// Prompt asks about the action in sentence case: `Delete label "bug" from alice/x?`. // Prompt asks about the action in sentence case: `Delete label "bug" from alice/x?`.
@@ -267,9 +262,3 @@ func ConfirmAction(ctx context.Context, c Confirmation, yes bool, streams Stream
} }
return err return err
} }
// Confirm requires explicit acceptance unless confirmation was supplied by flag.
// It is ConfirmAction for callers that predate Confirmation.
func Confirm(ctx context.Context, description string, yes bool, streams Streams, policy Policy, prompt Prompter) error {
return ConfirmAction(ctx, Confirmation{Prompt: description}, yes, streams, policy, prompt)
}
+3 -3
View File
@@ -56,12 +56,12 @@ func TestPromptPolicy(t *testing.T) {
} }
prompt := &fakePrompt{} prompt := &fakePrompt{}
value := "supplied" value := "supplied"
asked, err := Require(context.Background(), &value, "title", Streams{}, Policy{}, prompt) asked, err := RequireInput(context.Background(), &value, "title", "title", nil, Streams{}, Policy{}, prompt)
require.NoError(t, err) require.NoError(t, err)
require.False(t, asked) require.False(t, asked)
require.Zero(t, prompt.inputs) require.Zero(t, prompt.inputs)
value = "" value = ""
_, err = Require(context.Background(), &value, "title", Streams{}, Policy{}, prompt) _, err = RequireInput(context.Background(), &value, "title", "title", nil, Streams{}, Policy{}, prompt)
require.ErrorContains(t, err, "--title") require.ErrorContains(t, err, "--title")
} }
@@ -141,7 +141,7 @@ func TestConfirmActionPassesTheDefault(t *testing.T) {
prompt := &askingPrompt{accept: true} prompt := &askingPrompt{accept: true}
streams := Streams{InTTY: true, ErrTTY: true} streams := Streams{InTTY: true, ErrTTY: true}
require.NoError(t, ConfirmAction(context.Background(), Confirmation{Prompt: "Create pull request?", DefaultYes: true}, false, streams, Policy{Enabled: true}, prompt)) require.NoError(t, ConfirmAction(context.Background(), Confirmation{Prompt: "Create pull request?", DefaultYes: true}, false, streams, Policy{Enabled: true}, prompt))
require.NoError(t, Confirm(context.Background(), "Delete branch?", false, streams, Policy{Enabled: true}, prompt)) require.NoError(t, ConfirmAction(context.Background(), Confirmation{Prompt: "Delete branch?"}, false, streams, Policy{Enabled: true}, prompt))
require.Equal(t, []bool{true, false}, prompt.defaults) require.Equal(t, []bool{true, false}, prompt.defaults)
require.Equal(t, []string{"Create pull request?", "Delete branch?"}, prompt.titles) require.Equal(t, []string{"Create pull request?", "Delete branch?"}, prompt.titles)
} }
-35
View File
@@ -84,15 +84,6 @@ func (c Cell) style(text string, d Display, link bool) string {
return Hyperlink(c.Link, text, link && d.Links) return Hyperlink(c.Link, text, link && d.Links)
} }
// Row returns unstyled cells.
func Row(texts ...string) []Cell {
cells := make([]Cell, len(texts))
for i, text := range texts {
cells[i].Text = text
}
return cells
}
// Table renders rows under columns; rows may have more cells than there are columns. On a TTY it // Table renders rows under columns; rows may have more cells than there are columns. On a TTY it
// prints an uppercase header row when any column has a header, aligns columns two spaces apart, // prints an uppercase header row when any column has a header, aligns columns two spaces apart,
// and shrinks Flex columns until rows fit the display width. Otherwise it writes one line of // and shrinks Flex columns until rows fit the display width. Otherwise it writes one line of
@@ -209,32 +200,6 @@ func fitWidth(widths []int, limit int, flex func(j int) (bool, int)) {
} }
} }
// Record is one item for human readers: a heading line of cells separated by spaces,
// followed by text lines such as a body and a URL.
type Record struct {
Heading []Cell
Lines []string
}
// Render formats the record. Heading cells pass through SafeLine and are styled like
// Table cells; lines pass through SafeText, so they keep their own line breaks.
func (r Record) Render(color bool) string {
d := Display{TTY: true, Color: color, TrueColor: true, Links: color}
var b strings.Builder
for i, cell := range r.Heading {
if i > 0 {
b.WriteByte(' ')
}
b.WriteString(cell.style(SafeLine(cell.Text), d, true))
}
b.WriteByte('\n')
for _, line := range r.Lines {
b.WriteString(SafeText(line))
b.WriteByte('\n')
}
return b.String()
}
// SafeLine reduces server text to one line: SafeText, with tabs and line breaks as spaces. // SafeLine reduces server text to one line: SafeText, with tabs and line breaks as spaces.
func SafeLine(s string) string { func SafeLine(s string) string {
return strings.Map(func(r rune) rune { return strings.Map(func(r rune) rune {
+20 -20
View File
@@ -28,11 +28,11 @@ func TestTable(t *testing.T) {
want string want string
}{ }{
{"empty", issues, nil, tty, ""}, {"empty", issues, nil, tty, ""},
{"aligns legacy rows without header", nil, [][]Cell{Row("7", "Fix login", "open"), Row("12", "Crash", "closed")}, tty, "7 Fix login open\n12 Crash closed\n"}, {"aligns legacy rows without header", nil, [][]Cell{row("7", "Fix login", "open"), row("12", "Crash", "closed")}, tty, "7 Fix login open\n12 Crash closed\n"},
{"counts runes, not bytes", nil, [][]Cell{Row("1", "äöü", "x"), Row("2", "abcd", "y")}, tty, "1 äöü x\n2 abcd y\n"}, {"counts runes, not bytes", nil, [][]Cell{row("1", "äöü", "x"), row("2", "abcd", "y")}, tty, "1 äöü x\n2 abcd y\n"},
{"counts wide runes twice", nil, [][]Cell{Row("1", "修复", "x"), Row("2", "abcd", "y")}, tty, "1 修复 x\n2 abcd y\n"}, {"counts wide runes twice", nil, [][]Cell{row("1", "修复", "x"), row("2", "abcd", "y")}, tty, "1 修复 x\n2 abcd y\n"},
{"trims empty trailing cells", nil, [][]Cell{Row("1", "bug", ""), Row("22", "feature", "new work")}, tty, "1 bug\n22 feature new work\n"}, {"trims empty trailing cells", nil, [][]Cell{row("1", "bug", ""), row("22", "feature", "new work")}, tty, "1 bug\n22 feature new work\n"},
{"pads ragged rows", nil, [][]Cell{Row("1"), Row("2", "two")}, tty, "1\n2 two\n"}, {"pads ragged rows", nil, [][]Cell{row("1"), row("2", "two")}, tty, "1\n2 two\n"},
{ {
"heads columns and formats cells for people", "heads columns and formats cells for people",
issues, issueRows, tty, issues, issueRows, tty,
@@ -59,7 +59,7 @@ func TestTable(t *testing.T) {
{ {
"cuts wide runes by display width", "cuts wide runes by display width",
[]Column{{Header: "Title", Flex: true}, {Header: "By"}}, []Column{{Header: "Title", Flex: true}, {Header: "By"}},
[][]Cell{Row("支持中文路径 (support CJK paths)", "alice")}, [][]Cell{row("支持中文路径 (support CJK paths)", "alice")},
Display{TTY: true, Width: 20}, Display{TTY: true, Width: 20},
"TITLE BY\n支持中文路径… alice\n", "TITLE BY\n支持中文路径… alice\n",
}, },
@@ -73,21 +73,21 @@ func TestTable(t *testing.T) {
{ {
"keeps columns to their output", "keeps columns to their output",
[]Column{{Header: "ID"}, {Header: "State", PipedOnly: true}, {Header: "Link", TTYOnly: true}}, []Column{{Header: "ID"}, {Header: "State", PipedOnly: true}, {Header: "Link", TTYOnly: true}},
[][]Cell{Row("1", "open", "→")}, [][]Cell{row("1", "open", "→")},
tty, tty,
"ID LINK\n1 →\n", "ID LINK\n1 →\n",
}, },
{ {
"keeps piped-only columns when piped", "keeps piped-only columns when piped",
[]Column{{Header: "ID"}, {Header: "State", PipedOnly: true}, {Header: "Link", TTYOnly: true}}, []Column{{Header: "ID"}, {Header: "State", PipedOnly: true}, {Header: "Link", TTYOnly: true}},
[][]Cell{Row("1", "open", "→")}, [][]Cell{row("1", "open", "→")},
Display{}, Display{},
"1\topen\n", "1\topen\n",
}, },
{ {
"strips hostile text in both outputs", "strips hostile text in both outputs",
nil, nil,
[][]Cell{Row("1", "a\x1b]52;c;cHduZWQ=\x07b\x1b[2J\u009b31m‮txt.exe\r\ntail\tend", "open")}, [][]Cell{row("1", "a\x1b]52;c;cHduZWQ=\x07b\x1b[2J\u009b31m‮txt.exe\r\ntail\tend", "open")},
Display{}, Display{},
"1\ta]52;c;cHduZWQ=b[2J31mtxt.exe tail end\topen\n", "1\ta]52;c;cHduZWQ=b[2J31mtxt.exe tail end\topen\n",
}, },
@@ -95,7 +95,7 @@ func TestTable(t *testing.T) {
{ {
"styles after alignment and links only the first column", "styles after alignment and links only the first column",
[]Column{{Header: "ID"}, {Header: "Name"}, {Header: "State"}}, []Column{{Header: "ID"}, {Header: "Name"}, {Header: "State"}},
[][]Cell{{{Text: "7", Link: "https://example.org/7"}, {Text: "bug", Color: "ff0000", Link: "https://example.org/bug"}, {Text: "open", Color: "green", Bold: true}}, Row("12", "feature", "closed")}, [][]Cell{{{Text: "7", Link: "https://example.org/7"}, {Text: "bug", Color: "ff0000", Link: "https://example.org/bug"}, {Text: "open", Color: "green", Bold: true}}, row("12", "feature", "closed")},
Display{TTY: true, Color: true, Links: true}, Display{TTY: true, Color: true, Links: true},
"\x1b[90mID\x1b[0m \x1b[90mNAME\x1b[0m \x1b[90mSTATE\x1b[0m\n" + "\x1b[90mID\x1b[0m \x1b[90mNAME\x1b[0m \x1b[90mSTATE\x1b[0m\n" +
"\x1b]8;;https://example.org/7\x1b\\7\x1b]8;;\x1b\\ \x1b[38;5;196mbug\x1b[0m \x1b[1m\x1b[32mopen\x1b[0m\x1b[0m\n" + "\x1b]8;;https://example.org/7\x1b\\7\x1b]8;;\x1b\\ \x1b[38;5;196mbug\x1b[0m \x1b[1m\x1b[32mopen\x1b[0m\x1b[0m\n" +
@@ -126,7 +126,7 @@ func TestTableFitsWidthWithWideRunes(t *testing.T) {
titles := []string{"Emoji in title 🚀 works?", "支持中文路径 (support CJK paths)", strings.Repeat("long ", 40)} titles := []string{"Emoji in title 🚀 works?", "支持中文路径 (support CJK paths)", strings.Repeat("long ", 40)}
rows := make([][]Cell, len(titles)) rows := make([][]Cell, len(titles))
for i, title := range titles { for i, title := range titles {
rows[i] = Row("#1", title, "alice") rows[i] = row("#1", title, "alice")
} }
for _, width := range []int{30, 50, 80} { for _, width := range []int{30, 50, 80} {
for _, line := range strings.Split(strings.TrimSuffix(Table([]Column{{}, {Flex: true}, {}}, rows, Display{TTY: true, Width: width}), "\n"), "\n") { for _, line := range strings.Split(strings.TrimSuffix(Table([]Column{{}, {Flex: true}, {}}, rows, Display{TTY: true, Width: width}), "\n"), "\n") {
@@ -135,15 +135,6 @@ func TestTableFitsWidthWithWideRunes(t *testing.T) {
} }
} }
func TestRecord(t *testing.T) {
record := Record{
Heading: []Cell{{Text: "#7", Bold: true}, {Text: "Crash\x1b[2J\nnow"}, {Text: "bob‮"}},
Lines: []string{"Steps:\r\n\x1b]52;c;aGk=\x07run\n", "https://example.org/7"},
}
require.Equal(t, "#7 Crash[2J now bob\nSteps:\n]52;c;aGk=run\n\nhttps://example.org/7\n", record.Render(false))
require.Equal(t, "\x1b[1m#7\x1b[0m Crash[2J now bob\nSteps:\n]52;c;aGk=run\n\nhttps://example.org/7\n", record.Render(true))
}
func TestSafeText(t *testing.T) { func TestSafeText(t *testing.T) {
for _, tc := range []struct{ name, in, text, line string }{ for _, tc := range []struct{ name, in, text, line string }{
{"plain", "fix login", "fix login", "fix login"}, {"plain", "fix login", "fix login", "fix login"},
@@ -162,3 +153,12 @@ func TestSafeText(t *testing.T) {
}) })
} }
} }
// row returns unstyled cells.
func row(texts ...string) []Cell {
cells := make([]Cell, len(texts))
for i, text := range texts {
cells[i].Text = text
}
return cells
}