feat(app): report mutations on stderr and say what --web opens

Mutations printed a list row, or nothing, so a person could not tell
what changed and scripts parsed rows meant for people. Commands now get
the helpers for the gh convention:

- done and warn print "✓ Closed issue alice/x#5 (Fix login)" and
  "! … is already closed" to stderr, only when stdout is a terminal and
  --json is not set.
- printURL prints a created resource's URL to stdout unless --json is
  set, so url=$(tea issue create …) works.
- batchDone applies a mutation to several selections like batch and
  reports each success with a done line instead of a table.
- issueRef formats "alice/x#5 (title)" references.

--web now prints "Opening localhost:3001/alice/project/issues/1 in your
browser." to stderr on a terminal before the browser starts.

Signed-off-by: bircni <bircni@icloud.com>

internal/app/browser.go

@@ -3,12 +3,27 @@
 
 package app
 
-import "github.com/spf13/cobra"
+import (
+	"fmt"
+	"strings"
 
+	"gitea.dev/tea/internal/terminal"
+	"github.com/spf13/cobra"
+)
+
+// openBrowser opens resource in the configured browser, telling people on a terminal what opens.
 func (a *application) openBrowser(command *cobra.Command, resource string) error {
 	preferences, err := a.preferences()
 	if err != nil {
 		return err
 	}
+	if a.deps.Streams.OutTTY {
+		fmt.Fprintf(command.ErrOrStderr(), "Opening %s in your browser.\n", displayURL(resource))
+	}
 	return a.deps.Browser.Open(command.Context(), resource, preferences.Browser)
 }
+
+// displayURL shows a URL to people without its scheme, like gh.
+func displayURL(url string) string {
+	return terminal.SafeLine(strings.TrimPrefix(strings.TrimPrefix(url, "https://"), "http://"))
+}

internal/app/browser_test.go

@@ -68,3 +68,21 @@ func TestDiscussionViewsShareBrowserContract(t *testing.T) {
 		})
 	}
 }
+
+func TestOpenBrowserTellsTerminalUsers(t *testing.T) {
+	for _, tty := range []bool{true, false} {
+		deps, out, diagnostics := testDeps(t)
+		browser := &recordingBrowser{}
+		deps.Browser = browser
+		deps.Streams.OutTTY = tty
+		a, cmd := outputApplication(deps, "")
+		require.NoError(t, a.openBrowser(cmd, "http://localhost:3001/alice/project/issues/1"))
+		require.Equal(t, "http://localhost:3001/alice/project/issues/1", browser.resource)
+		require.Empty(t, out.String())
+		if tty {
+			require.Equal(t, "Opening localhost:3001/alice/project/issues/1 in your browser.\n", diagnostics.String())
+		} else {
+			require.Empty(t, diagnostics.String())
+		}
+	}
+}

internal/app/human_output.go

@@ -133,15 +133,48 @@ func writeView[T any](a *application, cmd *cobra.Command, item T, view func(T) t
 	return a.writeHuman(cmd, view(item).Render(a.display()))
 }
 
+// done reports a completed mutation on stderr, such as "✓ Closed issue alice/x#5 (Fix login)",
+// when people read stdout: on a TTY and without --json.
+func (a *application) done(cmd *cobra.Command, format string, args ...any) {
+	a.feedback(cmd, "✓", "green", format, args...)
+}
+
+// warn reports a mutation that had nothing to do, such as "! Issue alice/x#5 (Fix login) is already
+// closed", under the same conditions as done.
+func (a *application) warn(cmd *cobra.Command, format string, args ...any) {
+	a.feedback(cmd, "!", "yellow", format, args...)
+}
+
+func (a *application) feedback(cmd *cobra.Command, icon, color, format string, args ...any) {
+	if !a.deps.Streams.OutTTY || a.options.json != "" {
+		return
+	}
+	fmt.Fprintln(cmd.ErrOrStderr(), terminal.Colorize(color, icon, a.colorEnabled())+" "+terminal.SafeLine(fmt.Sprintf(format, args...)))
+}
+
+// printURL writes the URL of a created resource to stdout unless --json is set, so scripts can
+// capture it: url=$(tea issue create …).
+func (a *application) printURL(cmd *cobra.Command, url string) error {
+	if a.options.json != "" {
+		return nil
+	}
+	_, err := fmt.Fprintln(cmd.OutOrStdout(), terminal.SafeLine(url))
+	return err
+}
+
+// issueRef names an issue or pull request in feedback: "alice/x#5 (Fix login)".
+func issueRef(repo string, number int64, title string) string {
+	return fmt.Sprintf("%s#%d (%s)", repo, number, title)
+}
+
 // abortBatch stops a batch instead of recording an item failure.
 type abortBatch struct{ error }
 
 func (e abortBatch) Unwrap() error { return e.error }
 
-// batch applies run to each selection and continues past individual failures, which
+// applyBatch applies run to each selection and continues past individual failures, which it
-// it reports prefixed with the selection's label. Human output is one table of the
+// returns prefixed with the selection's label. succeeded sees each result as it arrives.
-// results; JSON is an object for one selection (none if it failed), otherwise an array.
+func applyBatch[K, T any](cmd *cobra.Command, keys []K, label func(K) string, run func(K) (T, error), succeeded func(T)) ([]T, []error) {
-func batch[K, T any](a *application, cmd *cobra.Command, keys []K, label func(K) string, run func(K) (T, error), row func(T) []terminal.Cell) error {
 	results := []T{}
 	var failures []error
 	for _, key := range keys {
@@ -159,15 +192,42 @@ func batch[K, T any](a *application, cmd *cobra.Command, keys []K, label func(K)
 			continue
 		}
 		results = append(results, item)
+		succeeded(item)
 	}
-	var err error
+	return results, failures
+}
+
+// writeBatchJSON writes an object for one selection (none if it failed), otherwise an array.
+func writeBatchJSON[T any](a *application, cmd *cobra.Command, selections int, results []T) error {
 	switch {
-	case a.options.json == "":
+	case selections != 1:
-		err = a.writeHuman(cmd, legacyTable(a, cmd, results, row))
+		return a.writeJSON(cmd, results)
-	case len(keys) != 1:
-		err = a.writeJSON(cmd, results)
 	case len(results) == 1:
-		err = a.writeJSON(cmd, results[0])
+		return a.writeJSON(cmd, results[0])
+	}
+	return nil
+}
+
+// batch applies run to each selection like batchDone, for commands that predate it: its human
+// output is one table of the results.
+func batch[K, T any](a *application, cmd *cobra.Command, keys []K, label func(K) string, run func(K) (T, error), row func(T) []terminal.Cell) error {
+	results, failures := applyBatch(cmd, keys, label, run, func(T) {})
+	var err error
+	if a.options.json == "" {
+		err = a.writeHuman(cmd, legacyTable(a, cmd, results, row))
+	} else {
+		err = writeBatchJSON(a, cmd, len(keys), results)
+	}
+	return errors.Join(append(failures, err)...)
+}
+
+// batchDone applies a mutation to each selection like batch, reporting each success with a done
+// line from report, such as "Closed issue alice/x#5 (Fix login)", and the results as JSON.
+func batchDone[K, T any](a *application, cmd *cobra.Command, keys []K, label func(K) string, run func(K) (T, error), report func(T) string) error {
+	results, failures := applyBatch(cmd, keys, label, run, func(item T) { a.done(cmd, "%s", report(item)) })
+	var err error
+	if a.options.json != "" {
+		err = writeBatchJSON(a, cmd, len(keys), results)
 	}
 	return errors.Join(append(failures, err)...)
 }
@@ -181,7 +241,8 @@ func writeList[T any](a *application, cmd *cobra.Command, items []T, row func(T)
 	return a.writeHuman(cmd, legacyTable(a, cmd, items, row))
 }
 
-// writeItem writes one item as a JSON object, or as its list row. Views move to writeView.
+// writeItem writes one item as a JSON object, or as its list row. Views move to writeView, and
+// mutations to done and printURL.
 func writeItem[T any](a *application, cmd *cobra.Command, item T, row func(T) []terminal.Cell) error {
 	if a.options.json != "" {
 		return a.writeJSON(cmd, item)

internal/app/human_output_test.go

@@ -6,6 +6,7 @@ package app
 import (
 	"bytes"
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
@@ -348,6 +349,66 @@ func TestWriteView(t *testing.T) {
 	}
 }
 
+func TestMutationFeedback(t *testing.T) {
+	for _, tc := range []struct {
+		name, json, out, stderr string
+		tty                     bool
+	}{
+		{name: "terminal", tty: true, out: "https://example.org/12\n", stderr: "✓ Created issue alice/x#12 (Fix[2J login)\n! Issue alice/x#12 (Fix[2J login) is already closed\n"},
+		{name: "piped", out: "https://example.org/12\n"},
+		{name: "JSON", tty: true, json: "number"},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			deps, out, diagnostics := testDeps(t)
+			deps.Streams.OutTTY = tc.tty
+			deps.Getenv = envFunc(map[string]string{"NO_COLOR": "1"})
+			a, cmd := outputApplication(deps, tc.json)
+			ref := issueRef("alice/x", 12, "Fix\x1b[2J login")
+			a.done(cmd, "Created issue %s", ref)
+			a.warn(cmd, "Issue %s is already closed", ref)
+			require.NoError(t, a.printURL(cmd, "https://example.org/12"))
+			require.Equal(t, tc.out, out.String())
+			require.Equal(t, tc.stderr, diagnostics.String())
+		})
+	}
+	deps, _, diagnostics := testDeps(t)
+	deps.Streams.OutTTY = true
+	a, cmd := outputApplication(deps, "")
+	a.done(cmd, "Closed issue %s", "alice/x#5 (Bug)")
+	a.warn(cmd, "nothing to do")
+	require.Equal(t, "\x1b[32m✓\x1b[0m Closed issue alice/x#5 (Bug)\n\x1b[33m!\x1b[0m nothing to do\n", diagnostics.String())
+}
+
+func TestBatchDoneReportsEachSuccess(t *testing.T) {
+	run := func(n int64) (listed, error) {
+		if n == 2 {
+			return listed{}, errors.New("not found")
+		}
+		return listed{Number: n, Title: fmt.Sprintf("Issue %d", n)}, nil
+	}
+	label := func(n int64) string { return fmt.Sprintf("#%d", n) }
+	report := func(item listed) string { return "Closed issue " + issueRef("alice/x", item.Number, item.Title) }
+	for _, tc := range []struct {
+		name, json, out, stderr string
+		tty                     bool
+	}{
+		{name: "terminal", tty: true, stderr: "✓ Closed issue alice/x#1 (Issue 1)\n✓ Closed issue alice/x#3 (Issue 3)\n"},
+		{name: "piped"},
+		{name: "JSON", json: "number", out: `[{"number":1},{"number":3}]` + "\n"},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			deps, out, diagnostics := testDeps(t)
+			deps.Streams.OutTTY = tc.tty
+			deps.Getenv = envFunc(map[string]string{"NO_COLOR": "1"})
+			a, cmd := outputApplication(deps, tc.json)
+			err := batchDone(a, cmd, []int64{1, 2, 3}, label, run, report)
+			require.EqualError(t, err, "#2: not found")
+			require.Equal(t, tc.out, out.String())
+			require.Equal(t, tc.stderr, diagnostics.String())
+		})
+	}
+}
+
 func TestShowing(t *testing.T) {
 	for _, tc := range []struct {
 		shown int