Skip to content

Commit

Permalink
pkg/cli: Write notifications with line wrapping.
Browse files Browse the repository at this point in the history
This allows a workaround for #1834.

This change is quite big because it changes the representation of the
notification from a term.Buffer to a ui.Text, which necessitates more downstream
changes.
  • Loading branch information
xiaq committed Oct 21, 2024
1 parent a0f27f9 commit 8527641
Show file tree
Hide file tree
Showing 16 changed files with 156 additions and 166 deletions.
19 changes: 9 additions & 10 deletions pkg/cli/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ func (a *app) redraw(flag redrawFlag) {
addons = append([]tk.Widget(nil), s.Addons...)
})

bufNotes := renderNotes(notes, width)
mergedNotes := mergeNotes(notes)
isFinalRedraw := flag&finalRedraw != 0
if isFinalRedraw {
hideRPrompt := !a.RPromptPersistent()
Expand All @@ -277,28 +277,27 @@ func (a *app) redraw(flag redrawFlag) {
// this with a Buffer that has one empty line.
bufMain.ExtendDown(&term.Buffer{Lines: [][]term.Cell{nil}}, true)

a.TTY.UpdateBuffer(bufNotes, bufMain, flag&fullRedraw != 0)
a.TTY.UpdateBuffer(mergedNotes, bufMain, flag&fullRedraw != 0)
a.TTY.ResetBuffer()
} else {
bufMain := renderApp(append([]tk.Widget{a.codeArea}, addons...), width, height)
a.TTY.UpdateBuffer(bufNotes, bufMain, flag&fullRedraw != 0)
a.TTY.UpdateBuffer(mergedNotes, bufMain, flag&fullRedraw != 0)
}
}

// Renders notes. This does not respect height so that overflow notes end up in
// the scrollback buffer.
func renderNotes(notes []ui.Text, width int) *term.Buffer {
// Merges notes, separating them with newlines.
func mergeNotes(notes []ui.Text) ui.Text {
if len(notes) == 0 {
return nil
}
bb := term.NewBufferBuilder(width)
tb := new(ui.TextBuilder)
for i, note := range notes {
if i > 0 {
bb.Newline()
tb.WriteText(ui.T("\n"))
}
bb.WriteStyled(note)
tb.WriteText(note)
}
return bb.Buffer()
return tb.Text()
}

// Renders the codearea, and uses the rest of the height for the listing.
Expand Down
7 changes: 3 additions & 4 deletions pkg/cli/app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,7 @@ func TestReadCode_NotifiesAboutUnboundKey(t *testing.T) {

f.TTY.Inject(term.K(ui.F1))

f.TestTTYNotes(t, "Unbound key: F1")
f.TTY.TestMsg(t, ui.T("Unbound key: F1"))
}

// Misc features.
Expand Down Expand Up @@ -531,9 +531,8 @@ func TestReadCode_ShowNotes(t *testing.T) {
f.App.Notify(ui.T("note 2"))
unblock <- struct{}{}

// Test that the note is rendered onto the notes buffer.
wantNotesBuf := bb().Write("note").Newline().Write("note 2").Buffer()
f.TTY.TestNotesBuffer(t, wantNotesBuf)
// Test that the message is rendered.
f.TTY.TestMsg(t, ui.T("note\nnote 2"))

// Test that notes are flushed after being rendered.
if n := len(f.App.CopyState().Notes); n > 0 {
Expand Down
6 changes: 0 additions & 6 deletions pkg/cli/clitest/apptest.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,12 +86,6 @@ func (f *Fixture) TestTTY(t *testing.T, args ...any) {
f.TTY.TestBuffer(t, f.MakeBuffer(args...))
}

// TestTTYNotes is equivalent to f.TTY.TestNotesBuffer(f.MakeBuffer(args...)).
func (f *Fixture) TestTTYNotes(t *testing.T, args ...any) {
t.Helper()
f.TTY.TestNotesBuffer(t, f.MakeBuffer(args...))
}

// StartReadCode starts the readCode function asynchronously, and returns two
// channels that deliver its return values. The two channels are closed after
// return values are delivered, so that subsequent reads will return zero values
Expand Down
2 changes: 1 addition & 1 deletion pkg/cli/clitest/apptest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func TestFixture(t *testing.T) {
f.TestTTY(t, "test", term.DotHere)

f.App.Notify(ui.T("something"))
f.TestTTYNotes(t, "something")
f.TTY.TestMsg(t, ui.T("something"))

f.App.CommitCode()
if code, err := f.Wait(); code != "test" || err != nil {
Expand Down
59 changes: 32 additions & 27 deletions pkg/cli/clitest/fake_tty.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"src.elv.sh/pkg/cli"
"src.elv.sh/pkg/cli/term"
"src.elv.sh/pkg/testutil"
"src.elv.sh/pkg/ui"
)

const (
Expand All @@ -31,9 +32,13 @@ type fakeTTY struct {
// Mutex for synchronizing writing and closing eventCh.
eventChMutex sync.Mutex
// Channel for publishing updates of the main buffer and notes buffer.
bufCh, notesBufCh chan *term.Buffer
bufCh chan *term.Buffer
// Channel for publishing updates of the message text.
msgCh chan ui.Text
// Records history of the main buffer and notes buffer.
bufs, notesBufs []*term.Buffer
bufs []*term.Buffer
// Records history of the message text.
msgs []ui.Text
// Mutexes for guarding bufs and notesBufs.
bufMutex sync.RWMutex
// Channel that NotifySignals returns. Can be used to inject signals.
Expand All @@ -59,11 +64,11 @@ const (
// size of the terminal is FakeTTYHeight and FakeTTYWidth.
func NewFakeTTY() (cli.TTY, TTYCtrl) {
tty := &fakeTTY{
eventCh: make(chan term.Event, fakeTTYEvents),
sigCh: make(chan os.Signal, fakeTTYSignals),
bufCh: make(chan *term.Buffer, fakeTTYBufferUpdates),
notesBufCh: make(chan *term.Buffer, fakeTTYBufferUpdates),
height: FakeTTYHeight, width: FakeTTYWidth,
eventCh: make(chan term.Event, fakeTTYEvents),
sigCh: make(chan os.Signal, fakeTTYSignals),
bufCh: make(chan *term.Buffer, fakeTTYBufferUpdates),
msgCh: make(chan ui.Text, fakeTTYBufferUpdates),
height: FakeTTYHeight, width: FakeTTYWidth,
}
return tty, TTYCtrl{tty}
}
Expand Down Expand Up @@ -118,10 +123,10 @@ func (t *fakeTTY) ResetBuffer() {

// UpdateBuffer records a new pair of buffers, i.e. sending them to their
// respective channels and appending them to their respective slices.
func (t *fakeTTY) UpdateBuffer(bufNotes, buf *term.Buffer, _ bool) error {
func (t *fakeTTY) UpdateBuffer(msg ui.Text, buf *term.Buffer, _ bool) error {
t.bufMutex.Lock()
defer t.bufMutex.Unlock()
t.recordNotesBuf(bufNotes)
t.recordMsg(msg)
t.recordBuf(buf)
return nil
}
Expand All @@ -145,9 +150,9 @@ func (t *fakeTTY) recordBuf(buf *term.Buffer) {
t.bufCh <- buf
}

func (t *fakeTTY) recordNotesBuf(buf *term.Buffer) {
t.notesBufs = append(t.notesBufs, buf)
t.notesBufCh <- buf
func (t *fakeTTY) recordMsg(msg ui.Text) {
t.msgs = append(t.msgs, msg)
t.msgCh <- msg
}

// TTYCtrl is an interface for controlling a fake terminal.
Expand Down Expand Up @@ -238,21 +243,21 @@ func (t TTYCtrl) TestBuffer(tt *testing.T, b *term.Buffer) {
}
}

// TestNotesBuffer verifies that a notes buffer will appear within 100ms, and
// aborts the test if it doesn't.
func (t TTYCtrl) TestNotesBuffer(tt *testing.T, b *term.Buffer) {
// TestNotesBuffer verifies that a message will appear within 100ms, and aborts
// the test if it doesn't.
func (t TTYCtrl) TestMsg(tt *testing.T, m ui.Text) {
tt.Helper()
ok := testBuffer(b, t.notesBufCh)
ok := testBuffer(m, t.msgCh)
if !ok {
tt.Logf("wanted notes buffer not shown:\n%s", b.TTYString())
tt.Logf("wanted notes buffer not shown:\n%s", m.VTString())

t.bufMutex.RLock()
defer t.bufMutex.RUnlock()
bufs := t.NotesBufferHistory()
tt.Logf("There has been %d notes buffers. None-nil ones are:", len(bufs))
bufs := t.MsgHistory()
tt.Logf("There has been %d messages. Non-nil ones are:", len(bufs))
for i, buf := range bufs {
if buf != nil {
tt.Logf("#%d:\n%s", i, buf.TTYString())
tt.Logf("#%d:\n%s", i, buf.VTString())
}
}
tt.FailNow()
Expand All @@ -277,23 +282,23 @@ func (t TTYCtrl) LastBuffer() *term.Buffer {
}

// NotesBufferHistory returns a slice of all notes buffers that have appeared.
func (t TTYCtrl) NotesBufferHistory() []*term.Buffer {
func (t TTYCtrl) MsgHistory() []ui.Text {
t.bufMutex.RLock()
defer t.bufMutex.RUnlock()
return t.notesBufs
return t.msgs
}

func (t TTYCtrl) LastNotesBuffer() *term.Buffer {
func (t TTYCtrl) LastMsg() ui.Text {
t.bufMutex.RLock()
defer t.bufMutex.RUnlock()
if len(t.notesBufs) == 0 {
if len(t.msgs) == 0 {
return nil
}
return t.notesBufs[len(t.notesBufs)-1]
return t.msgs[len(t.msgs)-1]
}

// Tests that an buffer appears on the channel within 100ms.
func testBuffer(want *term.Buffer, ch <-chan *term.Buffer) bool {
// Tests that a value appears on the channel within 100ms (scaled).
func testBuffer[T any](want T, ch <-chan T) bool {
timeout := time.After(testutil.Scaled(100 * time.Millisecond))
for {
select {
Expand Down
35 changes: 18 additions & 17 deletions pkg/cli/clitest/fake_tty_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

"src.elv.sh/pkg/cli"
"src.elv.sh/pkg/cli/term"
"src.elv.sh/pkg/ui"
)

func TestFakeTTY_Setup(t *testing.T) {
Expand Down Expand Up @@ -67,54 +68,54 @@ func TestFakeTTY_Signals(t *testing.T) {
}

func TestFakeTTY_Buffer(t *testing.T) {
bufNotes1 := term.NewBufferBuilder(10).Write("notes 1").Buffer()
msg1 := ui.T("msg 1")
buf1 := term.NewBufferBuilder(10).Write("buf 1").Buffer()
bufNotes2 := term.NewBufferBuilder(10).Write("notes 2").Buffer()
msg2 := ui.T("msg 2")
buf2 := term.NewBufferBuilder(10).Write("buf 2").Buffer()
bufNotes3 := term.NewBufferBuilder(10).Write("notes 3").Buffer()
msg3 := ui.T("msg 3")
buf3 := term.NewBufferBuilder(10).Write("buf 3").Buffer()

tty, ttyCtrl := NewFakeTTY()

if ttyCtrl.LastNotesBuffer() != nil {
t.Errorf("LastNotesBuffer -> %v, want nil", ttyCtrl.LastNotesBuffer())
if ttyCtrl.LastMsg() != nil {
t.Errorf("LastNotesBuffer -> %v, want nil", ttyCtrl.LastMsg())
}
if ttyCtrl.LastBuffer() != nil {
t.Errorf("LastBuffer -> %v, want nil", ttyCtrl.LastBuffer())
}

tty.UpdateBuffer(bufNotes1, buf1, true)
if ttyCtrl.LastNotesBuffer() != bufNotes1 {
t.Errorf("LastBuffer -> %v, want %v", ttyCtrl.LastNotesBuffer(), bufNotes1)
tty.UpdateBuffer(msg1, buf1, true)
if !reflect.DeepEqual(ttyCtrl.LastMsg(), msg1) {
t.Errorf("LastBuffer -> %v, want %v", ttyCtrl.LastMsg(), msg1)
}
if ttyCtrl.LastBuffer() != buf1 {
t.Errorf("LastBuffer -> %v, want %v", ttyCtrl.LastBuffer(), buf1)
}
ttyCtrl.TestBuffer(t, buf1)
ttyCtrl.TestNotesBuffer(t, bufNotes1)
ttyCtrl.TestMsg(t, msg1)

tty.UpdateBuffer(bufNotes2, buf2, true)
if ttyCtrl.LastNotesBuffer() != bufNotes2 {
t.Errorf("LastBuffer -> %v, want %v", ttyCtrl.LastNotesBuffer(), bufNotes2)
tty.UpdateBuffer(msg2, buf2, true)
if !reflect.DeepEqual(ttyCtrl.LastMsg(), msg2) {
t.Errorf("LastBuffer -> %v, want %v", ttyCtrl.LastMsg(), msg2)
}
if ttyCtrl.LastBuffer() != buf2 {
t.Errorf("LastBuffer -> %v, want %v", ttyCtrl.LastBuffer(), buf2)
}
ttyCtrl.TestBuffer(t, buf2)
ttyCtrl.TestNotesBuffer(t, bufNotes2)
ttyCtrl.TestMsg(t, msg2)

// Test Test{,Notes}Buffer
tty.UpdateBuffer(bufNotes3, buf3, true)
tty.UpdateBuffer(msg3, buf3, true)
ttyCtrl.TestBuffer(t, buf3)
ttyCtrl.TestNotesBuffer(t, bufNotes3)
ttyCtrl.TestMsg(t, msg3)
// Cannot test the failure branch as that will fail the test

wantBufs := []*term.Buffer{buf1, buf2, buf3}
wantNotesBufs := []*term.Buffer{bufNotes1, bufNotes2, bufNotes3}
wantMsgs := []ui.Text{msg1, msg2, msg3}
if !reflect.DeepEqual(ttyCtrl.BufferHistory(), wantBufs) {
t.Errorf("BufferHistory did not return {buf1, buf2}")
}
if !reflect.DeepEqual(ttyCtrl.NotesBufferHistory(), wantNotesBufs) {
if !reflect.DeepEqual(ttyCtrl.MsgHistory(), wantMsgs) {
t.Errorf("NotesBufferHistory did not return {bufNotes1, bufNotes2}")
}
}
Expand Down
8 changes: 2 additions & 6 deletions pkg/cli/modes/histwalk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,7 @@ func TestHistWalk_NoWalker(t *testing.T) {
defer f.Stop()

startHistwalk(f.App, HistwalkSpec{})
f.TestTTYNotes(t,
"error: no history store", Styles,
"!!!!!!")
f.TTY.TestMsg(t, ui.Concat(ui.T("error:", ui.FgRed), ui.T(" no history store")))
}

func TestHistWalk_NoMatch(t *testing.T) {
Expand All @@ -99,9 +97,7 @@ func TestHistWalk_NoMatch(t *testing.T) {
cfg := HistwalkSpec{Store: store, Prefix: "ls"}
startHistwalk(f.App, cfg)
// Test that an error message has been written to the notes buffer.
f.TestTTYNotes(t,
"error: end of history", Styles,
"!!!!!!")
f.TTY.TestMsg(t, ui.Concat(ui.T("error:", ui.FgRed), ui.T(" end of history")))
// Test that buffer has not changed - histwalk addon is not active.
f.TTY.TestBuffer(t, buf0)
}
Expand Down
5 changes: 2 additions & 3 deletions pkg/cli/modes/location_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,8 @@ func TestLocation_FullWorkflow(t *testing.T) {
// There should be no change to codearea after accepting.
f.TestTTY(t /* nothing */)
// Error from Chdir should be sent to notes.
f.TestTTYNotes(t,
"error: mock chdir error", Styles,
"!!!!!!")
f.TTY.TestMsg(t, ui.Concat(
ui.T("error:", ui.FgRed), ui.T(" mock chdir error")))
// Chdir should be called.
wantChdir := fixPath("/tmp/foo/bar/lorem/ipsum")
select {
Expand Down
10 changes: 4 additions & 6 deletions pkg/cli/modes/navigation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,8 @@ func TestErrorInAscend(t *testing.T) {
startNavigation(f.App, NavigationSpec{Cursor: c})

f.TTY.Inject(term.K(ui.Left))
f.TestTTYNotes(t,
"error: cannot ascend", Styles,
"!!!!!!")
f.TTY.TestMsg(t, ui.Concat(
ui.T("error:", ui.FgRed), ui.T(" cannot ascend")))
}

func TestErrorInDescend(t *testing.T) {
Expand All @@ -53,9 +52,8 @@ func TestErrorInDescend(t *testing.T) {

f.TTY.Inject(term.K(ui.Down))
f.TTY.Inject(term.K(ui.Right))
f.TestTTYNotes(t,
"error: cannot descend", Styles,
"!!!!!!")
f.TTY.TestMsg(t, ui.Concat(
ui.T("error:", ui.FgRed), ui.T(" cannot descend")))
}

func TestErrorInCurrent(t *testing.T) {
Expand Down
Loading

0 comments on commit 8527641

Please sign in to comment.