Skip to content

Commit 03668f2

Browse files
author
gluck
committed
the glyph stands alone, and the alert actually leaves
NAMES ARE VERBOSE-ONLY, as instructed the first time. I had this right, then "corrected" it against a sketch that was showing the verbose form -- reading a picture over an instruction. This bar is read by someone who knows the glyphs; spelling every symbol out by default spends the row's width teaching a lesson its only reader has already had. THE ALERT NOW RETIRES, and the reason it did not is not subtle: setNoticeTTL existed, cli.notice_ttl existed, and NOTHING CALLED EITHER. Every alert was born with a zero TTL, which means "hold the slot until displaced" -- so an error sat in the bar until the next one arrived, which in practice is forever. A default a caller has to install is not a default; it is now on the constructor. And the ticker is not the only clock: an idle pager animates nothing, so building the view checks expiry too. One comparison, where the wall clock is already being read. Proven the way it should have been the first time: a real command posts a real alert in a real pty, and the script waits out the full ten seconds and looks again. Two unit tests drive the clock rather than sleeping. ONLY TROUBLE IS RED. A confirmation wears the row's own gray; alertLevel is the same severity notifications will sort by, so it is one field rather than two ideas. THE PIT'S HEIGHT NO LONGER MOVES. The "… N more" markers were drawn on top of the window instead of out of its budget, so crossing the top of a list changed the pit's height by a row and jogged the transcript above it. The markers are now reserved from the same budget and the pit draws exactly the rows it was given. The '!' panel's leading blank row is gone -- it read as an extra newline under the rule. STILL WRONG, AND I KNOW WHY NOW: `form show`/`form listen` keeps its own rows, cursor and top (fifteen references in form_listen.go) and its own j/k handler -- a FOURTH list implementation, beside the picker, that the pit hosts through cmdkit.LiveView without ever seeing its list. Its window is computed as height-2 while the pit reserves its own, so the cursor and the visible range disagree and the highlight appears to skip entries. Making it an `act` over the picker is the fix, and it is the same consolidation that unblocks fullscreen.
1 parent bfa2b5a commit 03668f2

9 files changed

Lines changed: 242 additions & 44 deletions

File tree

‎internal/cli/command.go‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,13 +51,22 @@ func (in *interactiveInput) commandAsync(fn func(context.Context) (string, error
5151
defer cancel()
5252
msg, err := fn(ctx)
5353
if err != nil {
54-
in.note(err.Error())
54+
in.noteErr(err.Error())
5555
return
5656
}
5757
in.note(msg)
5858
}()
5959
}
6060

61+
// noteErr is note for TROUBLE: same slot, same retirement, red rather than
62+
// gray. The split is here because this is the one place that knows which it
63+
// was -- the verb returned an error or it did not.
64+
func (in *interactiveInput) noteErr(msg string) {
65+
in.mu.Lock()
66+
in.lt.tr.setCommandNoteAt(msg, alertError)
67+
in.mu.Unlock()
68+
}
69+
6170
// note writes one line into the pager's status row, under the render lock.
6271
func (in *interactiveInput) note(msg string) {
6372
in.mu.Lock()

‎internal/cli/drawerid.go‎

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -104,25 +104,26 @@ func (d drawerID) face() drawerFace {
104104
// a keyMode from what is on screen.
105105
func (d drawerID) keys() keyMode { return d.face().keys }
106106

107-
// token is what the pit contributes to the left of the status bar: GLYPH AND
108-
// NAME, always, because that is what the design draws --
107+
// token is what the pit contributes to the left of the status bar: THE GLYPH
108+
// ALONE, and the name only under `m`.
109109
//
110-
// 𝄚 queue · ✓ · 123abc · test 9.8k/1.0m (1.0%)
110+
// 𝄚 · ✓ · 123abc · test 9.8k/1.0m (1.0%) (default)
111+
// 𝄚 queue · done ✓ · 123abc · test · … (with `m`)
111112
//
112-
// It is not a verbose extra. The name is how you know which pit you are in,
113-
// and hiding it behind a toggle made the succinct bar say only "𝄚", which
114-
// answers the question for someone who has already memorised the glyphs and
115-
// for nobody else. Empty for the pits with nothing to announce, so the row
116-
// then begins with the state.
117-
func (d drawerID) token() string {
113+
// This bar is read by someone who knows the glyphs. Verbose is for the reader
114+
// who does not, and spelling every symbol out by default spends the row's
115+
// width teaching a lesson its only reader has already had. (I had this right,
116+
// then "corrected" it against a sketch that was showing the verbose form.)
117+
func (d drawerID) token(verbose bool) string {
118118
f := d.face()
119-
if f.glyph == "" {
119+
switch {
120+
case f.glyph == "":
120121
return ""
121-
}
122-
if f.name == "" {
122+
case verbose && f.name != "":
123+
return f.glyph + " " + f.name
124+
default:
123125
return f.glyph
124126
}
125-
return f.glyph + " " + f.name
126127
}
127128

128129
// selectionGlyph marks the row under the cursor. It is PER DRAWER, not one

‎internal/cli/picker.go‎

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -204,14 +204,32 @@ func (p *picker) remove() {
204204

205205
// lines draws the window: the visible rows, the selection marker, and an
206206
// honest count of what is out of view on either side.
207+
// lines draws EXACTLY h rows, always. The markers are part of the budget, not
208+
// an addition to it: they appear and vanish as you scroll, and a pit whose
209+
// height moved by a row every time you crossed the top of the list made the
210+
// transcript above it jump by a row too. A window that changes size while you
211+
// read it is the thing this component exists to stop.
207212
func (p *picker) lines(id drawerID, w, h int) []string {
208-
p.height = clampInt(h, 1, pickerRows)
213+
budget := clampInt(h, 1, pickerRows)
214+
// Reserve the marker rows FIRST, out of the same budget.
215+
markAbove, markBelow := 0, 0
216+
if len(p.rows) > budget {
217+
// A list that does not fit shows at least one marker; which one
218+
// depends on where the window sits, so both are reserved and the
219+
// visible count is fixed either way.
220+
markAbove, markBelow = 1, 1
221+
}
222+
p.height = max(budget-markAbove-markBelow, 1)
209223
p.top = clampInt(p.top, 0, p.maxTop())
210224
end := min(p.top+p.window(), len(p.rows))
211225

212-
out := make([]string, 0, p.window()+2)
213-
if p.top > 0 {
214-
out = append(out, drawerGray(clipToWidth(" "+AndMore(p.top, "above"), w)))
226+
out := make([]string, 0, budget)
227+
if markAbove > 0 {
228+
if p.top > 0 {
229+
out = append(out, drawerGray(clipToWidth(" "+AndMore(p.top, "above"), w)))
230+
} else {
231+
out = append(out, "")
232+
}
215233
}
216234
mark := id.selectionGlyph()
217235
for i := p.top; i < end; i++ {
@@ -226,8 +244,12 @@ func (p *picker) lines(id drawerID, w, h int) []string {
226244
}
227245
out = append(out, row)
228246
}
229-
if rest := len(p.rows) - end; rest > 0 {
230-
out = append(out, drawerGray(clipToWidth(" "+AndMore(rest, ""), w)))
247+
if markBelow > 0 {
248+
if rest := len(p.rows) - end; rest > 0 {
249+
out = append(out, drawerGray(clipToWidth(" "+AndMore(rest, ""), w)))
250+
} else {
251+
out = append(out, "")
252+
}
231253
}
232254
return out
233255
}

‎internal/cli/session_status.go‎

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,12 @@ type sessionStatus struct {
113113
lastAt time.Time
114114
// model is shown under verbose only.
115115
model string
116+
// noticeLevel is what KIND of news the alert is. "sent" and "showing
117+
// abc12345" are confirmations and wear the row's own gray; only trouble is
118+
// red. Painting every alert red made the bar cry wolf on its own success
119+
// messages -- and it is the same severity the notification ring will sort
120+
// by, so it is one field rather than two ideas.
121+
noticeLevel alertLevel
116122
// noticeUntil is when the bar's alert retires; see retireAlert.
117123
noticeUntil time.Time
118124
// noticeTTL is how long a posted alert holds the slot. Zero: forever.
@@ -126,8 +132,18 @@ type sessionStatus struct {
126132
notice string
127133
}
128134

135+
// defaultNoticeTTL is how long an alert holds the bar's first slot. It is a
136+
// CONSTRUCTOR DEFAULT, not a config-only value, and that distinction is the
137+
// whole of a bug I shipped: setNoticeTTL existed, config.NoticeTTL existed,
138+
// and nothing called either -- so every alert was minted with a zero TTL,
139+
// which means "hold the slot until something displaces you", which means an
140+
// error sat in the bar until the next one arrived. Forever, in practice.
141+
//
142+
// A default that has to be installed by a caller is not a default.
143+
const defaultNoticeTTL = 10 * time.Second
144+
129145
func newSessionStatus(figaroID string, startedAt time.Time) *sessionStatus {
130-
return &sessionStatus{figaroID: figaroID, startedAt: startedAt}
146+
return &sessionStatus{figaroID: figaroID, startedAt: startedAt, noticeTTL: defaultNoticeTTL}
131147
}
132148

133149
// setNotice publishes (or clears, with "") the bar's alert: the newest thing
@@ -140,12 +156,33 @@ func newSessionStatus(figaroID string, startedAt time.Time) *sessionStatus {
140156
// dismissed: a bar item that must be acknowledged is a bar item that is still
141157
// there tomorrow. Retirement happens on the ticker (retireAlert), because a
142158
// clock cannot fire inside a pure renderer.
159+
// alertLevel sorts news by whether it is trouble.
160+
type alertLevel uint8
161+
162+
const (
163+
alertInfo alertLevel = iota // a confirmation: gray, like the rest of the row
164+
alertError // trouble: red, and eventually a notification
165+
)
166+
167+
// setNoticeAt posts an alert with its level. setNotice is the confirmation
168+
// case, which is the common one.
169+
func (s *sessionStatus) setNoticeAt(text string, level alertLevel) {
170+
s.setNotice(text)
171+
if s == nil {
172+
return
173+
}
174+
s.mu.Lock()
175+
s.noticeLevel = level
176+
s.mu.Unlock()
177+
}
178+
143179
func (s *sessionStatus) setNotice(text string) {
144180
if s == nil {
145181
return
146182
}
147183
s.mu.Lock()
148184
s.notice = strings.Join(strings.Fields(text), " ")
185+
s.noticeLevel = alertInfo
149186
switch {
150187
case s.notice == "":
151188
s.noticeUntil = time.Time{}
@@ -316,8 +353,9 @@ func (s *sessionStatus) panelLines() []string {
316353
if state == "" {
317354
state = "idle 𝄐"
318355
}
356+
// NO LEADING BLANK. The panel used to open with an empty row, which read as
357+
// an extra newline under the rule above it.
319358
rows := []string{
320-
"",
321359
" aria " + s.figaroID,
322360
" status " + state,
323361
}

‎internal/cli/session_status_notice_test.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,10 @@ func TestStatusNoticeIsRedAndLeftmost(t *testing.T) {
5252
// routing it through the palette fixes.
5353
defer term.SetColorMode(term.ColorAlways)()
5454

55+
// ONLY TROUBLE IS RED now, so this test posts trouble. A confirmation
56+
// ("sent") wears the row's own gray -- see TestOnlyTroubleIsRed.
5557
s := statusFixture("a perfectly ordinary mantra")
56-
s.setNotice("error: overloaded")
58+
s.setNoticeAt("error: overloaded", alertError)
5759
line := row(s, 200)
5860

5961
// Through the palette, not a literal: the assertion asks the theme what a

‎internal/cli/statusview.go‎

Lines changed: 31 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,8 @@ type statusView struct {
3232
// TRANSIENT and its expiry is evaluated by whoever builds this value --
3333
// never here, or the pure renderer becomes a function of the wall clock
3434
// and its goldens become flaky.
35-
Alert string
35+
Alert string
36+
AlertLevel alertLevel
3637
// LastAt is when the conversation last moved. Verbose only, and absolute:
3738
// a relative "3m ago" would be true only at the instant it was painted.
3839
LastAt time.Time
@@ -89,13 +90,21 @@ func (v statusView) render(w int) []string {
8990
// groups is the split: what is about the MODE and the conversation on the left,
9091
// what is a fact about the aria on the right.
9192
func (v statusView) groups() (left, right []string) {
92-
// The alert leads, ahead of even the mode: trouble belongs where the eye
93-
// lands first, and it is the only field that arrives without being asked
94-
// for.
93+
// The alert leads, ahead of even the pit: news belongs where the eye lands
94+
// first, and it is the only field that arrives without being asked for.
95+
//
96+
// ONLY TROUBLE IS RED. "sent" is a confirmation and wears the row's own
97+
// gray; painting every alert red made the bar shout its successes in the
98+
// colour it reserves for failures, which is how a colour stops meaning
99+
// anything.
95100
if v.Alert != "" {
96-
left = append(left, term.NoticeInDim(v.Alert))
101+
if v.AlertLevel == alertError {
102+
left = append(left, term.NoticeInDim(v.Alert))
103+
} else {
104+
left = append(left, v.Alert)
105+
}
97106
}
98-
if tok := v.Drawer.token(); tok != "" {
107+
if tok := v.Drawer.token(v.Verbose); tok != "" {
99108
left = append(left, tok)
100109
}
101110
if st := v.stateToken(); st != "" {
@@ -154,17 +163,25 @@ func (s *sessionStatus) viewOf(drawer drawerID, verbose bool, now time.Time) sta
154163
if s == nil {
155164
return statusView{}
156165
}
166+
// THE TICKER IS NOT THE ONLY CLOCK. It runs while something animates; an
167+
// idle pager animates nothing, so an alert posted after the turn ended
168+
// would have waited for the next keystroke to notice it had expired. Every
169+
// build of the view checks too -- it is one comparison, it happens exactly
170+
// where the wall clock is already being read, and it makes "the alert
171+
// retires" true rather than usually true.
172+
s.retireAlert(now)
157173
s.mu.RLock()
158174
defer s.mu.RUnlock()
159175
v := statusView{
160-
Drawer: drawer,
161-
State: s.turn,
162-
Tick: s.tick,
163-
Aria: s.figaroID,
164-
Mantra: s.metrics.Mantra,
165-
Alert: s.notice,
166-
LastAt: s.lastAt,
167-
Verbose: verbose,
176+
Drawer: drawer,
177+
State: s.turn,
178+
Tick: s.tick,
179+
Aria: s.figaroID,
180+
Mantra: s.metrics.Mantra,
181+
Alert: s.notice,
182+
AlertLevel: s.noticeLevel,
183+
LastAt: s.lastAt,
184+
Verbose: verbose,
168185
}
169186
if ctx := formatContextUsage(s.metrics.ContextTokens, s.metrics.ContextLimit, s.metrics.ContextExact); ctx != "-" {
170187
v.Ctx = ctx

‎internal/cli/statusview_test.go‎

Lines changed: 90 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import (
44
"strings"
55
"testing"
66
"time"
7+
8+
"github.com/jack-work/figaro/internal/term"
79
)
810

911
// ---------------------------------------------------------------------------
@@ -42,12 +44,11 @@ func TestStatusViewGoldens(t *testing.T) {
4244
want: []string{"✓ · 123abc · test 9.8k/1.0m (1.0%)"},
4345
},
4446
{
45-
// THE NAME IS NOT A VERBOSE EXTRA. The design draws "𝄚 queue",
46-
// always: the glyph alone answers the question only for someone
47-
// who has already memorised the glyphs.
48-
name: "a pit leads with its glyph AND its name", width: 80,
47+
// THE NAME IS A VERBOSE EXTRA. This bar is read by someone who
48+
// knows the glyphs; the word is for the reader who does not.
49+
name: "a pit leads with its glyph alone", width: 80,
4950
view: func(v statusView) statusView { v.Drawer = drawerQueue; return v },
50-
want: []string{"𝄚 queue · ✓ · 123abc · test 9.8k/1.0m (1.0%)"},
51+
want: []string{"𝄚 · ✓ · 123abc · test 9.8k/1.0m (1.0%)"},
5152
},
5253
{
5354
name: "verbose names the drawer and the state", width: 96,
@@ -171,3 +172,87 @@ func TestVerboseCarriesTheLastInteraction(t *testing.T) {
171172
t.Fatalf("the default bar carries a datetime: %q", row)
172173
}
173174
}
175+
176+
// TestOnlyTroubleIsRed: a confirmation wears the row's own gray. Painting
177+
// every alert red made the bar shout "sent" in the colour it keeps for
178+
// failures, which is how a colour stops meaning anything.
179+
func TestOnlyTroubleIsRed(t *testing.T) {
180+
restore := term.SetColorMode(term.ColorAlways)
181+
defer restore()
182+
183+
v := barFixture()
184+
v.Alert = "sent"
185+
info := strings.Join(v.render(100), "")
186+
if !strings.Contains(info, "sent") {
187+
t.Fatalf("the confirmation is missing: %q", info)
188+
}
189+
if strings.Contains(info, term.NoticeInDim("sent")) {
190+
t.Fatalf("a confirmation was painted as trouble: %q", info)
191+
}
192+
193+
v.AlertLevel = alertError
194+
bad := strings.Join(v.render(100), "")
195+
if !strings.Contains(bad, term.NoticeInDim("sent")) {
196+
t.Fatalf("trouble was not painted red: %q", bad)
197+
}
198+
}
199+
200+
// TestAlertLevelResetsOnEveryPost: a red error must not tint the confirmation
201+
// that follows it. setNotice is the confirmation door and clears the level.
202+
func TestAlertLevelResetsOnEveryPost(t *testing.T) {
203+
s := newSessionStatus("aria1234", time.Now())
204+
s.setNoticeAt("boom", alertError)
205+
if v := s.viewOf(drawerNothing, false, time.Now()); v.AlertLevel != alertError {
206+
t.Fatal("the error did not take")
207+
}
208+
s.setNotice("sent")
209+
if v := s.viewOf(drawerNothing, false, time.Now()); v.AlertLevel != alertInfo {
210+
t.Fatal("a confirmation inherited the previous error's colour")
211+
}
212+
}
213+
214+
// TestAlertRetiresOnItsOwn is the test that was missing, and its absence is
215+
// why an alert sat in the bar forever: setNoticeTTL existed, cli.notice_ttl
216+
// existed, and NOTHING CALLED EITHER -- so every alert was born with a zero
217+
// TTL, which means "hold the slot until displaced".
218+
//
219+
// It drives the clock rather than sleeping: the expiry is a comparison against
220+
// a time the caller passes, precisely so this can be tested in microseconds.
221+
func TestAlertRetiresOnItsOwn(t *testing.T) {
222+
s := newSessionStatus("aria1234", time.Now())
223+
if s.noticeTTL <= 0 {
224+
t.Fatal("a fresh session has no notice TTL; a default a caller must install is not a default")
225+
}
226+
s.setNotice("sent")
227+
228+
now := time.Now()
229+
if v := s.viewOf(drawerNothing, false, now); v.Alert != "sent" {
230+
t.Fatalf("the alert did not post: %q", v.Alert)
231+
}
232+
// Still there a moment later.
233+
if v := s.viewOf(drawerNothing, false, now.Add(time.Second)); v.Alert != "sent" {
234+
t.Fatalf("the alert retired early: %q", v.Alert)
235+
}
236+
// And gone once its span is up, WITHOUT a keystroke or a tick: an idle
237+
// pager animates nothing, so the view build is the backstop.
238+
if v := s.viewOf(drawerNothing, false, now.Add(defaultNoticeTTL+time.Second)); v.Alert != "" {
239+
t.Fatalf("the alert outlived its TTL: %q", v.Alert)
240+
}
241+
}
242+
243+
// TestNewerAlertDisplacesOlder: the other way an alert leaves. A burst of
244+
// them -- ten `:send`s in a row -- must show the newest, not a stack.
245+
func TestNewerAlertDisplacesOlder(t *testing.T) {
246+
s := newSessionStatus("aria1234", time.Now())
247+
for i := 0; i < 10; i++ {
248+
s.setNotice("sent")
249+
}
250+
s.setNotice("showing abc12345")
251+
v := s.viewOf(drawerNothing, false, time.Now())
252+
if v.Alert != "showing abc12345" {
253+
t.Fatalf("the newest alert did not win: %q", v.Alert)
254+
}
255+
if strings.Count(v.Alert, "sent") != 0 {
256+
t.Fatalf("alerts stacked instead of displacing: %q", v.Alert)
257+
}
258+
}

0 commit comments

Comments
 (0)