diff --git a/internal/app/download.go b/internal/app/download.go index dbecb1ea..7e560c78 100644 --- a/internal/app/download.go +++ b/internal/app/download.go @@ -19,7 +19,6 @@ import ( "gitea.dev/tea/internal/clierr" "gitea.dev/tea/internal/terminal" "github.com/spf13/cobra" - "golang.org/x/term" ) // 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. const binarySniffLength = 8 << 10 diff --git a/internal/app/download_test.go b/internal/app/download_test.go index e7549a87..2168a27b 100644 --- a/internal/app/download_test.go +++ b/internal/app/download_test.go @@ -40,21 +40,24 @@ func TestDownloadsPublishOnlyCompleteFiles(t *testing.T) { cmd.SetContext(context.Background()) var stdout bytes.Buffer 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()) directory := t.TempDir() 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) require.True(t, errors.Is(err, os.ErrNotExist)) entries, err := os.ReadDir(directory) require.NoError(t, err) 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) require.NoError(t, err) 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) require.NoError(t, err) require.Equal(t, "archive", string(data)) diff --git a/internal/services/pulls.go b/internal/services/pulls.go index 2a32361f..bbfac283 100644 --- a/internal/services/pulls.go +++ b/internal/services/pulls.go @@ -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. func (s Pulls) Create(ctx context.Context, opts PullOptions) (Pull, error) { // TODO(sdk): CreatePullRequestOption lacks allow_maintainer_edit. diff --git a/internal/services/pulls_test.go b/internal/services/pulls_test.go index e228b9ba..4cd5742d 100644 --- a/internal/services/pulls_test.go +++ b/internal/services/pulls_test.go @@ -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) { assert.Equal(t, "/api/v1/repos/alice/project/pulls", r.URL.Path) switch r.URL.Query().Get("page") { @@ -70,11 +70,10 @@ func TestFindOpenByHeadPagesPastSmallServerLimitsAndMatchesExactly(t *testing.T) } })) 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.EqualValues(t, 2, item.Number) - _, err = service.FindOpenByHead(context.Background(), "FEATURE") - require.ErrorContains(t, err, "no open pull request") + require.Len(t, items, 2) + require.Equal(t, "feature", items[1].Head) } func TestListOpenRejectsEndlessPagination(t *testing.T) { diff --git a/internal/terminal/streams.go b/internal/terminal/streams.go index 107b40ba..767f3c14 100644 --- a/internal/terminal/streams.go +++ b/internal/terminal/streams.go @@ -205,11 +205,6 @@ func RequireInput(ctx context.Context, value *string, flag, label string, valida 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. type Confirmation struct { // 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 } - -// 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) -} diff --git a/internal/terminal/streams_test.go b/internal/terminal/streams_test.go index 502dfceb..a80212b3 100644 --- a/internal/terminal/streams_test.go +++ b/internal/terminal/streams_test.go @@ -56,12 +56,12 @@ func TestPromptPolicy(t *testing.T) { } prompt := &fakePrompt{} 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.False(t, asked) require.Zero(t, prompt.inputs) 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") } @@ -141,7 +141,7 @@ func TestConfirmActionPassesTheDefault(t *testing.T) { prompt := &askingPrompt{accept: 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, 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, []string{"Create pull request?", "Delete branch?"}, prompt.titles) } diff --git a/internal/terminal/table.go b/internal/terminal/table.go index c1584957..c3cab655 100644 --- a/internal/terminal/table.go +++ b/internal/terminal/table.go @@ -84,15 +84,6 @@ func (c Cell) style(text string, d Display, link bool) string { 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 // 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 @@ -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. func SafeLine(s string) string { return strings.Map(func(r rune) rune { diff --git a/internal/terminal/table_test.go b/internal/terminal/table_test.go index ee113460..7b57099e 100644 --- a/internal/terminal/table_test.go +++ b/internal/terminal/table_test.go @@ -28,11 +28,11 @@ func TestTable(t *testing.T) { want string }{ {"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"}, - {"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"}, - {"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"}, + {"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 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"}, + {"pads ragged rows", nil, [][]Cell{row("1"), row("2", "two")}, tty, "1\n2 two\n"}, { "heads columns and formats cells for people", issues, issueRows, tty, @@ -59,7 +59,7 @@ func TestTable(t *testing.T) { { "cuts wide runes by display width", []Column{{Header: "Title", Flex: true}, {Header: "By"}}, - [][]Cell{Row("支持中文路径 (support CJK paths)", "alice")}, + [][]Cell{row("支持中文路径 (support CJK paths)", "alice")}, Display{TTY: true, Width: 20}, "TITLE BY\n支持中文路径… alice\n", }, @@ -73,21 +73,21 @@ func TestTable(t *testing.T) { { "keeps columns to their output", []Column{{Header: "ID"}, {Header: "State", PipedOnly: true}, {Header: "Link", TTYOnly: true}}, - [][]Cell{Row("1", "open", "→")}, + [][]Cell{row("1", "open", "→")}, tty, "ID LINK\n1 →\n", }, { "keeps piped-only columns when piped", []Column{{Header: "ID"}, {Header: "State", PipedOnly: true}, {Header: "Link", TTYOnly: true}}, - [][]Cell{Row("1", "open", "→")}, + [][]Cell{row("1", "open", "→")}, Display{}, "1\topen\n", }, { "strips hostile text in both outputs", 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{}, "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", []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}, "\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" + @@ -126,7 +126,7 @@ func TestTableFitsWidthWithWideRunes(t *testing.T) { titles := []string{"Emoji in title 🚀 works?", "支持中文路径 (support CJK paths)", strings.Repeat("long ", 40)} rows := make([][]Cell, len(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 _, 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) { for _, tc := range []struct{ name, in, text, line string }{ {"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 +}