diff --git a/internal/cli/app.go b/internal/cli/app.go index 207048b..e1b1f91 100644 --- a/internal/cli/app.go +++ b/internal/cli/app.go @@ -108,7 +108,7 @@ func (a *App) handleInput(text string) { }) if err != nil { a.ui.AddStatus( - "[red]Send error: " + err.Error(), + "[red]Send error: " + escape(err.Error()), ) } } @@ -166,7 +166,7 @@ func (a *App) dispatchInfoCommand(cmd, args string) { a.cmdWhois(args) default: a.ui.AddStatus( - "[red]Unknown command: " + cmd, + "[red]Unknown command: " + escape(cmd), ) } } @@ -182,7 +182,7 @@ func (a *App) cmdConnect(serverURL string) { serverURL = strings.TrimRight(serverURL, "/") - a.ui.AddStatus("Connecting to " + serverURL + "...") + a.ui.AddStatus("Connecting to " + escape(serverURL) + "...") a.mu.Lock() nick := a.nick @@ -192,9 +192,9 @@ func (a *App) cmdConnect(serverURL string) { resp, err := client.CreateSession(nick) if err != nil { - a.ui.AddStatus(fmt.Sprintf( - "[red]Connection failed: %v", err, - )) + a.ui.AddStatus( + "[red]Connection failed: " + escape(err.Error()), + ) return } @@ -218,7 +218,7 @@ func (a *App) cmdConnect(serverURL string) { a.ui.AddStatus(fmt.Sprintf( "[green]Connected! Nick: %s, Session: %d", - resp.Nick, resp.ID, + escape(resp.Nick), resp.ID, )) a.ui.SetStatus(resp.Nick, "", "connected") @@ -244,7 +244,7 @@ func (a *App) cmdNick(nick string) { a.mu.Unlock() a.ui.AddStatus( - "Nick set to " + nick + + "Nick set to " + escape(nick) + " (will be used on connect)", ) @@ -256,9 +256,9 @@ func (a *App) cmdNick(nick string) { Body: []string{nick}, }) if err != nil { - a.ui.AddStatus(fmt.Sprintf( - "[red]Nick change failed: %v", err, - )) + a.ui.AddStatus( + "[red]Nick change failed: " + escape(err.Error()), + ) return } @@ -269,7 +269,7 @@ func (a *App) cmdNick(nick string) { a.mu.Unlock() a.ui.SetStatus(nick, target, "connected") - a.ui.AddStatus("Nick changed to " + nick) + a.ui.AddStatus("Nick changed to " + escape(nick)) } func (a *App) cmdJoin(channel string) { @@ -297,9 +297,9 @@ func (a *App) cmdJoin(channel string) { err := a.client.JoinChannel(channel) if err != nil { - a.ui.AddStatus(fmt.Sprintf( - "[red]Join failed: %v", err, - )) + a.ui.AddStatus( + "[red]Join failed: " + escape(err.Error()), + ) return } @@ -337,9 +337,9 @@ func (a *App) cmdPart(channel string) { err := a.client.PartChannel(channel) if err != nil { - a.ui.AddStatus(fmt.Sprintf( - "[red]Part failed: %v", err, - )) + a.ui.AddStatus( + "[red]Part failed: " + escape(err.Error()), + ) return } @@ -384,9 +384,9 @@ func (a *App) cmdMsg(args string) { Body: []string{text}, }) if err != nil { - a.ui.AddStatus(fmt.Sprintf( - "[red]Send failed: %v", err, - )) + a.ui.AddStatus( + "[red]Send failed: " + escape(err.Error()), + ) } } @@ -439,9 +439,9 @@ func (a *App) cmdTopic(args string) { Body: []string{args}, }) if err != nil { - a.ui.AddStatus(fmt.Sprintf( - "[red]Topic set failed: %v", err, - )) + a.ui.AddStatus( + "[red]Topic set failed: " + escape(err.Error()), + ) } } @@ -465,9 +465,9 @@ func (a *App) cmdNames() { members, err := a.client.GetMembers(target) if err != nil { - a.ui.AddStatus(fmt.Sprintf( - "[red]Names failed: %v", err, - )) + a.ui.AddStatus( + "[red]Names failed: " + escape(err.Error()), + ) return } @@ -480,7 +480,7 @@ func (a *App) cmdNames() { a.ui.AddLine(target, fmt.Sprintf( "[cyan]*** Members of %s: %s", - target, strings.Join(nicks, " "), + escape(target), escape(strings.Join(nicks, " ")), )) } @@ -511,9 +511,9 @@ func (a *App) cmdList() { channels, err := a.client.ListChannels() if err != nil { - a.ui.AddStatus(fmt.Sprintf( - "[red]List failed: %v", err, - )) + a.ui.AddStatus( + "[red]List failed: " + escape(err.Error()), + ) return } @@ -522,7 +522,7 @@ func (a *App) cmdList() { for _, ch := range channels { a.ui.AddStatus(fmt.Sprintf( - " %s %s", ch.Name, ch.Topic, + " %s %s", escape(ch.Name), escape(ch.Topic), )) } @@ -544,9 +544,9 @@ func (a *App) cmdMotd() { &api.Message{Command: irc.CmdMotd}, //nolint:exhaustruct ) if err != nil { - a.ui.AddStatus(fmt.Sprintf( - "[red]MOTD failed: %v", err, - )) + a.ui.AddStatus( + "[red]MOTD failed: " + escape(err.Error()), + ) } } @@ -582,9 +582,9 @@ func (a *App) cmdWho(args string) { }, ) if err != nil { - a.ui.AddStatus(fmt.Sprintf( - "[red]WHO failed: %v", err, - )) + a.ui.AddStatus( + "[red]WHO failed: " + escape(err.Error()), + ) } } @@ -613,9 +613,9 @@ func (a *App) cmdWhois(args string) { }, ) if err != nil { - a.ui.AddStatus(fmt.Sprintf( - "[red]WHOIS failed: %v", err, - )) + a.ui.AddStatus( + "[red]WHOIS failed: " + escape(err.Error()), + ) } } @@ -818,7 +818,7 @@ func (a *App) handlePrivmsgEvent( a.ui.AddLine(target, fmt.Sprintf( "[gray]%s [green]<%s>[white] %s", - timestamp, msg.From, text, + timestamp, escape(msg.From), escape(text), )) } @@ -831,7 +831,7 @@ func (a *App) handleJoinEvent( a.ui.AddLine(msg.To, fmt.Sprintf( "[gray]%s [yellow]*** %s has joined %s", - timestamp, msg.From, msg.To, + timestamp, escape(msg.From), escape(msg.To), )) } @@ -850,11 +850,11 @@ func (a *App) handlePartEvent( line := fmt.Sprintf( "[gray]%s [yellow]*** %s has left %s", - timestamp, msg.From, msg.To, + timestamp, escape(msg.From), escape(msg.To), ) if reason != "" { - line += " (" + reason + ")" + line += " (" + escape(reason) + ")" } if msg.From == myNick { @@ -875,12 +875,12 @@ func (a *App) handleQuitEvent( if reason != "" { a.ui.AddStatus(fmt.Sprintf( "[gray]%s [yellow]*** %s has quit (%s)", - timestamp, msg.From, reason, + timestamp, escape(msg.From), escape(reason), )) } else { a.ui.AddStatus(fmt.Sprintf( "[gray]%s [yellow]*** %s has quit", - timestamp, msg.From, + timestamp, escape(msg.From), )) } } @@ -907,7 +907,7 @@ func (a *App) handleNickEvent( a.ui.AddStatus(fmt.Sprintf( "[gray]%s [yellow]*** %s is now known as %s", - timestamp, msg.From, newNick, + timestamp, escape(msg.From), escape(newNick), )) } @@ -919,7 +919,7 @@ func (a *App) handleNoticeEvent( a.ui.AddStatus(fmt.Sprintf( "[gray]%s [magenta]--%s-- %s", - timestamp, msg.From, text, + timestamp, escape(msg.From), escape(text), )) } @@ -935,7 +935,7 @@ func (a *App) handleTopicEvent( a.ui.AddLine(msg.To, fmt.Sprintf( "[gray]%s [cyan]*** %s set topic: %s", - timestamp, msg.From, text, + timestamp, escape(msg.From), escape(text), )) } @@ -945,10 +945,12 @@ func (a *App) handleDefaultEvent( lines := msg.BodyLines() text := strings.Join(lines, " ") + // The brackets around the command are escaped with it, as tview + // reads a command such as "[MODE]" as a tag. if text != "" { a.ui.AddStatus(fmt.Sprintf( - "[gray]%s [white][%s] %s", - timestamp, msg.Command, text, + "[gray]%s [white]%s %s", + timestamp, escape("["+msg.Command+"]"), escape(text), )) } } diff --git a/internal/cli/app_internal_test.go b/internal/cli/app_internal_test.go index 679e67e..0ef43d7 100644 --- a/internal/cli/app_internal_test.go +++ b/internal/cli/app_internal_test.go @@ -8,6 +8,7 @@ import ( "strings" "testing" "time" + "unicode/utf8" "github.com/gdamore/tcell/v2" api "sneak.berlin/go/neoirc/internal/cli/api" @@ -845,6 +846,109 @@ func TestOwnEventFromOtherClient(t *testing.T) { } } +// TestBracketsInMessageWindow checks that each kind of line drawn +// from a server event shows nicks, texts, reasons and topics +// holding square brackets as written. +func TestBracketsInMessageWindow(t *testing.T) { + t.Parallel() + + // Every event is sent to the channel and has a body of one line. + tests := []struct { + name string + command string + from string + body string + // Whether the line goes to the channel's window rather than + // the status window, which is the one shown at start. + inChannel bool + // How the line ends, after the timestamps. + want string + }{ + { + name: "message", command: irc.CmdPrivmsg, inChannel: true, + from: "bob[|[]", body: "x[^[] [:::y|z] [red]", + want: " x[^[] [:::y|z] [red]", + }, + { + name: "joined", command: irc.CmdJoin, from: "x[^[]", inChannel: true, + want: "*** x[^[] has joined #chan", + }, + { + name: "left", command: irc.CmdPart, inChannel: true, + from: "erin[away]", body: "[:::y|z]", + want: "*** erin[away] has left #chan ([:::y|z])", + }, + { + name: "topic", command: irc.CmdTopic, inChannel: true, + from: "dave[afk]", body: "[red]news", + want: "*** dave[afk] set topic: [red]news", + }, + { + name: "quit", command: irc.CmdQuit, from: "carol[x]", body: "bye [red]", + want: "*** carol[x] has quit (bye [red])", + }, + { + name: "nick change", command: irc.CmdNick, from: "erin[m]", body: "erin[x]", + want: "*** erin[m] is now known as erin[x]", + }, + { + name: "notice", command: irc.CmdNotice, from: "bob[]", body: "[::b]note", + want: "--bob[]-- [::b]note", + }, + { + name: "other command", command: irc.CmdMode, body: "+v dave[afk]", + want: "[MODE] +v dave[afk]", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + app := &App{ui: NewUI(), nick: testNick} + screen := runUI(t, app.ui) + + if test.inChannel { + app.ui.SwitchToBuffer(testChannel) + } + + app.handleServerMessage(&api.Message{ + Command: test.command, From: test.from, To: testChannel, + Body: []string{test.body}, + }) + + got, _ := shownLine(app.ui, screen) + if !strings.HasSuffix(got, test.want) { + t.Fatalf("message window shows %q, want it to end %q", got, test.want) + } + }) + } +} + +// TestColourTagInMessage checks that a colour tag in a message +// shows as written and leaves the message text white, as the +// client draws it. +func TestColourTagInMessage(t *testing.T) { + t.Parallel() + + const want = " [red]x" + + app := &App{ui: NewUI(), nick: testNick} + screen := runUI(t, app.ui) + app.ui.SwitchToBuffer(testChannel) + + app.handleServerMessage(&api.Message{ + Command: irc.CmdPrivmsg, From: testOtherNick, To: testChannel, + Body: []string{"[red]x"}, + }) + + got, colour := shownLine(app.ui, screen) + if !strings.HasSuffix(got, want) || colour != tcell.ColorWhite { + t.Fatalf("message window shows %q, its last character in %v; "+ + "want it to end %q, in white", got, colour, want) + } +} + // bufferState returns the number of lines in the named // buffer and how many of them are unread. func bufferState(ui *UI, name string) (int, int) { @@ -872,6 +976,39 @@ func statusBarText(ui *UI) string { return text } +// shownLine returns the first line of the message window as the +// screen shows it, without the blanks after it, and the colour of +// its last character. +func shownLine( + ui *UI, screen tcell.SimulationScreen, +) (string, tcell.Color) { + var ( + text string + colour tcell.Color + ) + + ui.app.QueueUpdate(func() { + cells, width, _ := screen.GetContents() + + var row strings.Builder + + for _, cell := range cells[:width] { + row.WriteString(string(cell.Runes)) + } + + text = strings.TrimRight(row.String(), " ") + if text == "" { + return + } + + // Each character in these tests takes one cell. + last := cells[utf8.RuneCountInString(text)-1] + colour, _, _ = last.Style.Decompose() + }) + + return text, colour +} + // handledLines makes app.ui pass each typed line to // app.handleInput, as Run does, and returns a channel that // receives each line once it has been handled. @@ -964,11 +1101,15 @@ func waitPoll(t *testing.T, polled <-chan struct{}, server string) { } } -// runUI runs ui on a simulated screen until the test ends. -func runUI(t *testing.T, ui *UI) { +// runUI runs ui on a simulated screen until the test ends, and +// returns the screen. +// +//nolint:ireturn // tcell has no exported type for the screen +func runUI(t *testing.T, ui *UI) tcell.SimulationScreen { t.Helper() - ui.app.SetScreen(tcell.NewSimulationScreen("")) + screen := tcell.NewSimulationScreen("") + ui.app.SetScreen(screen) // One slot, so that Run's goroutine can end even after the // cleanup below has stopped waiting for it. @@ -988,4 +1129,6 @@ func runUI(t *testing.T, ui *UI) { t.Error("the UI did not stop: its event loop stopped responding") } }) + + return screen } diff --git a/internal/cli/ui.go b/internal/cli/ui.go index 6f229df..80708a6 100644 --- a/internal/cli/ui.go +++ b/internal/cli/ui.go @@ -235,6 +235,16 @@ func (ui *UI) initMessages() { ui.messages.SetBorder(false) } +// escape returns text so that the message window shows it exactly +// as written. It writes each "[" as "[[:]": a "[" followed by +// another "[" never starts a tag, and the tag "[:]" changes +// nothing. tview.Escape is not enough: it does not escape a bracket +// pair holding a symbol such as "|" or "^", and nothing it adds +// stops a tag with a URL, such as "[:::y|z]". +func escape(text string) string { + return strings.ReplaceAll(text, "[", "[[:]") +} + func (ui *UI) initStatusBar() { // Colour tags stay off. With them on, tview reads text in // square brackets, such as "[connected]" or part of a nick,