From f8141b8f8d87a3c82743c6dca634dd0bd55edd72 Mon Sep 17 00:00:00 2001 From: ayn2op Date: Fri, 16 Oct 2026 18:52:15 -0400 Subject: [PATCH] refactor: break up long inline expressions --- internal/config/ui.go | 25 +++++++++++----- internal/ui/chat/composer/model.go | 11 ++++++- internal/ui/chat/guildstree/model.go | 10 +++++-- internal/ui/chat/memberstree/model.go | 22 ++++++++++---- internal/ui/chat/messageslist/model.go | 32 ++++++++++++++------- internal/ui/chat/messageslist/model_test.go | 6 ++-- internal/ui/chat/messageslist/preview.go | 6 +++- internal/ui/chat/model.go | 8 ++++-- internal/ui/tree/tree.go | 11 ++++++- 9 files changed, 98 insertions(+), 33 deletions(-) diff --git a/internal/config/ui.go b/internal/config/ui.go index 14accb4..8cc9249 100644 --- a/internal/config/ui.go +++ b/internal/config/ui.go @@ -229,13 +229,24 @@ func defaultUI() UIConfig { Arrows: [2]string{"◀", "▶"}, ClickableArrows: true, }, - Title: left, - Footer: left, - Border: BorderUIConfig{Enabled: true, Padding: [4]int{0, 0, 1, 1}, NormalSet: round, ActiveSet: round}, - GuildsTree: GuildsTreeUIConfig{TreeUIConfig{Graphics: true}, true}, - MembersTree: TreeUIConfig{Graphics: true}, - ScrollBar: ScrollBarUIConfig{SymbolSet: SymbolSetWrapper{boxDrawing}, Visibility: ScrollBarVisibilityWrapper{viewport.ScrollBarVisibilityAutomatic}}, - MessagesList: MessagesListUIConfig{Spacing: 1, ReplyIndicator: ">", ForwardedIndicator: "<", ImageProtocol: "auto", Attachments: attachments, Avatars: AvatarsUIConfig{Preview: true, Height: 2}, Grouping: GroupingUIConfig{Enabled: true, Interval: 10 * time.Minute}}, + Title: left, + Footer: left, + Border: BorderUIConfig{Enabled: true, Padding: [4]int{0, 0, 1, 1}, NormalSet: round, ActiveSet: round}, + GuildsTree: GuildsTreeUIConfig{TreeUIConfig{Graphics: true}, true}, + MembersTree: TreeUIConfig{Graphics: true}, + ScrollBar: ScrollBarUIConfig{ + SymbolSet: SymbolSetWrapper{boxDrawing}, + Visibility: ScrollBarVisibilityWrapper{viewport.ScrollBarVisibilityAutomatic}, + }, + MessagesList: MessagesListUIConfig{ + Spacing: 1, + ReplyIndicator: ">", + ForwardedIndicator: "<", + ImageProtocol: "auto", + Attachments: attachments, + Avatars: AvatarsUIConfig{Preview: true, Height: 2}, + Grouping: GroupingUIConfig{Enabled: true, Interval: 10 * time.Minute}, + }, MentionsList: MentionsListUIConfig{MinWidth: 20}, } } diff --git a/internal/ui/chat/composer/model.go b/internal/ui/chat/composer/model.go index 74e552e..c4ba002 100644 --- a/internal/ui/chat/composer/model.go +++ b/internal/ui/chat/composer/model.go @@ -187,7 +187,16 @@ func (m Model) footer() string { // isBound reports whether key is one of the composer's keybinds. func (m Model) isBound(key tview.KeyMsg) bool { k := m.cfg.Keybinds.Composer - if keybind.Matches(key, k.Paste.Keybind, k.Send.Keybind, k.OpenEditor.Keybind, k.OpenFilePicker.Keybind, k.Cancel.Keybind, k.TabComplete.Keybind, k.ToggleReplyMention.Keybind, k.Undo.Keybind) { + if keybind.Matches(key, + k.Paste.Keybind, + k.Send.Keybind, + k.OpenEditor.Keybind, + k.OpenFilePicker.Keybind, + k.Cancel.Keybind, + k.TabComplete.Keybind, + k.ToggleReplyMention.Keybind, + k.Undo.Keybind, + ) { return true } return keybind.Matches(key, k.EditLast.Keybind) && m.canEditLastMessage() diff --git a/internal/ui/chat/guildstree/model.go b/internal/ui/chat/guildstree/model.go index 6faec51..14ce9f3 100644 --- a/internal/ui/chat/guildstree/model.go +++ b/internal/ui/chat/guildstree/model.go @@ -223,7 +223,8 @@ func (m Model) createGuildNode(parent *tree.Node, guild discord.Guild) { } func (m Model) createChannelNode(parent *tree.Node, channel discord.Channel) { - if channel.Type != discord.DirectMessage && channel.Type != discord.GroupDM && channel.Type != discord.GuildCategory && !m.state.HasPermissions(channel.ID, discord.PermissionViewChannel) { + needsViewPermission := !slices.Contains([]discord.ChannelType{discord.DirectMessage, discord.GroupDM, discord.GuildCategory}, channel.Type) + if needsViewPermission && !m.state.HasPermissions(channel.ID, discord.PermissionViewChannel) { return } @@ -372,11 +373,14 @@ func (m Model) voiceChannel(node *tree.Node) *discord.Channel { // joinable returns why the voice channel of node cannot be joined, or nil if it can be. func (m Model) joinable(node *tree.Node, channel discord.Channel) error { + // The users in the channel are the children of its node. + atUserLimit := channel.VoiceUserLimit > 0 && len(node.Children()) >= int(channel.VoiceUserLimit) + switch { case !m.state.HasPermissions(channel.ID, discord.PermissionConnect): return errors.New("you are not permitted to connect") - // Discord does not answer a request to join a full voice channel. - case channel.VoiceUserLimit > 0 && len(node.Children()) >= int(channel.VoiceUserLimit) && !m.state.HasPermissions(channel.ID, discord.PermissionMoveMembers): + // Discord does not answer a request to join a full voice channel, unless it is from someone who can move members. + case atUserLimit && !m.state.HasPermissions(channel.ID, discord.PermissionMoveMembers): return errors.New("it is full") } diff --git a/internal/ui/chat/memberstree/model.go b/internal/ui/chat/memberstree/model.go index 018fa1f..cce3662 100644 --- a/internal/ui/chat/memberstree/model.go +++ b/internal/ui/chat/memberstree/model.go @@ -87,7 +87,19 @@ func (m *Model) SetChannel(channel *discord.Channel) tview.Cmd { func (m Model) View(focused bool) tview.Widget { onChange := func(current *tree.Node) tview.Msg { return Msg{current} } onSelect := func(n *tree.Node) tview.Msg { return SelectedMsg{Node: n} } - return uitree.New(m.root, m.current, m.scrollState, m.cfg, m.cfg.Theme.MembersTree, m.cfg.UI.MembersTree.Graphics, m.cfg.Keybinds.MembersTree, focused, onChange, onSelect).Title("Members") + return uitree.New( + m.root, + m.current, + m.scrollState, + m.cfg, + m.cfg.Theme.MembersTree, + m.cfg.UI.MembersTree.Graphics, + m.cfg.Keybinds.MembersTree, + focused, + onChange, + onSelect, + ). + Title("Members") } // setCurrentNode selects node and scrolls to it. @@ -202,11 +214,11 @@ func (m Model) addRecipients(recipients []discord.User) { if me, err := m.state.Cabinet.Me(); err == nil { recipients = append(recipients, *me) } + rank := func(user discord.User) int { return statusRank(m.status(discord.NullGuildID, user.ID)) } + name := func(user discord.User) string { return strings.ToLower(user.DisplayOrUsername()) } + // By status, online first, then by name. slices.SortFunc(recipients, func(a, b discord.User) int { - if c := cmp.Compare(statusRank(m.status(discord.NullGuildID, a.ID)), statusRank(m.status(discord.NullGuildID, b.ID))); c != 0 { - return c - } - return strings.Compare(strings.ToLower(a.DisplayOrUsername()), strings.ToLower(b.DisplayOrUsername())) + return cmp.Or(cmp.Compare(rank(a), rank(b)), strings.Compare(name(a), name(b))) }) group := m.addGroup(discord.NullGuildID, gateway.GuildMemberListGroup{ID: "members", Count: uint64(len(recipients))}) diff --git a/internal/ui/chat/messageslist/model.go b/internal/ui/chat/messageslist/model.go index 69a01ae..dcf5f3f 100644 --- a/internal/ui/chat/messageslist/model.go +++ b/internal/ui/chat/messageslist/model.go @@ -216,12 +216,18 @@ func (m *Model) setMessages(messages []discord.Message) { func (m *Model) AddMessage(message discord.Message) tview.Cmd { // Appending keeps the items before it as they are, so only the separator of a new day is added rather than all rebuilt. - last := len(m.items) - 1 - separator := m.cfg.DateSeparator.Enabled && (last < 0 || !sameLocalDate(m.items[last].message.Timestamp, message.Timestamp)) + var previous *discord.Message + if last := len(m.items) - 1; last >= 0 { + previous = &m.items[last].message + } + + separator := m.shouldSeparate(previous, message) if separator { m.items = append(m.items, messageItem{separator: true}) } - m.items = append(m.items, messageItem{message: message, grouped: last >= 0 && !separator && m.grouped(m.items[last].message, message)}) + + grouped := !separator && m.shouldGroup(previous, message) + m.items = append(m.items, messageItem{message: message, grouped: grouped}) return m.loadPreviews() } @@ -388,10 +394,10 @@ func (m Model) renderMessage(message discord.Message, baseStyle tview.Style, wid return text } -// grouped reports whether message is drawn under the header of previous, the message before it. -func (m Model) grouped(previous, message discord.Message) bool { +// shouldGroup reports whether message is drawn under the header of previous, the message before it, which is nil for the first. +func (m Model) shouldGroup(previous *discord.Message, message discord.Message) bool { cfg := m.cfg.UI.MessagesList.Grouping - return cfg.Enabled && + return cfg.Enabled && previous != nil && // Replies and system messages start a group, as their header says more than who wrote them. message.Type == discord.DefaultMessage && (previous.Type == discord.DefaultMessage || previous.Type == discord.InlinedReplyMessage) && @@ -404,17 +410,18 @@ func (m Model) grouped(previous, message discord.Message) bool { func (m *Model) rebuildItems() { items := make([]messageItem, 0, len(m.items)) cursor, target := -1, -1 - var previous discord.Message + var previous *discord.Message for i, item := range m.items { if item.separator { continue } - separator := m.cfg.DateSeparator.Enabled && (len(items) == 0 || !sameLocalDate(previous.Timestamp, item.message.Timestamp)) + + separator := m.shouldSeparate(previous, item.message) if separator { items = append(items, messageItem{separator: true}) } // A message that joins or leaves a group is drawn again, with or without its header. - if grouped := len(items) > 0 && !separator && m.grouped(previous, item.message); grouped != item.grouped { + if grouped := !separator && m.shouldGroup(previous, item.message); grouped != item.grouped { item.grouped, item.text = grouped, nil } if i == m.cursor { @@ -424,13 +431,18 @@ func (m *Model) rebuildItems() { target = len(items) } items = append(items, item) - previous = item.message + previous = &item.message } m.items = items // The selected and target messages only move with the separators, so they are not scrolled to. m.cursor, m.target = cursor, target } +// shouldSeparate reports whether a date separator goes before message, as it is the first (previous is nil) or starts a new day. +func (m Model) shouldSeparate(previous *discord.Message, message discord.Message) bool { + return m.cfg.DateSeparator.Enabled && (previous == nil || !sameLocalDate(previous.Timestamp, message.Timestamp)) +} + func sameLocalDate(a discord.Timestamp, b discord.Timestamp) bool { ta := a.Time().In(time.Local) tb := b.Time().In(time.Local) diff --git a/internal/ui/chat/messageslist/model_test.go b/internal/ui/chat/messageslist/model_test.go index 8597897..b859fda 100644 --- a/internal/ui/chat/messageslist/model_test.go +++ b/internal/ui/chat/messageslist/model_test.go @@ -234,7 +234,7 @@ func TestModelDrawLayout(t *testing.T) { } } -func TestModelGrouped(t *testing.T) { +func TestModelShouldGroup(t *testing.T) { start := time.Date(2026, 9, 1, 12, 0, 0, 0, time.UTC) message := func(author string, after time.Duration, typ discord.MessageType) discord.Message { return discord.Message{Type: typ, Author: discord.User{ID: 2, Username: author}, Timestamp: discord.NewTimestamp(start.Add(after)), Content: "hi"} @@ -254,7 +254,7 @@ func TestModelGrouped(t *testing.T) { {"after a system message", message("a", 0, discord.ChannelPinnedMessage), message("a", time.Minute, discord.DefaultMessage), false}, } { t.Run(tt.name, func(t *testing.T) { - if got := newTestModel(t, testConfig(t)).grouped(tt.previous, tt.message); got != tt.want { + if got := newTestModel(t, testConfig(t)).shouldGroup(&tt.previous, tt.message); got != tt.want { t.Fatalf("grouped = %v, want %v", got, tt.want) } }) @@ -263,7 +263,7 @@ func TestModelGrouped(t *testing.T) { cfg := testConfig(t) cfg.Timestamps.Enabled = false ml := newTestModel(t, cfg) - if cfg.UI.MessagesList.Grouping.Enabled = false; ml.grouped(previous, previous) { + if cfg.UI.MessagesList.Grouping.Enabled = false; ml.shouldGroup(&previous, previous) { t.Fatal("grouped while disabled") } cfg.UI.MessagesList.Grouping.Enabled = true diff --git a/internal/ui/chat/messageslist/preview.go b/internal/ui/chat/messageslist/preview.go index 995e09a..7672e4d 100644 --- a/internal/ui/chat/messageslist/preview.go +++ b/internal/ui/chat/messageslist/preview.go @@ -58,7 +58,11 @@ func previewSources(message discord.Message) []previewSource { // cols returns the preview's width in cells of cellWidth by cellHeight pixels: its own width at most, within the preview box. func (s previewSource) cols(cellWidth, cellHeight uint) int { - return int(max(min((s.width+cellWidth-1)/cellWidth, previewWidth, s.width*previewHeight*cellHeight/(s.height*cellWidth)), 1)) + // The columns its own width takes, rounded up, so it is not scaled up. + own := (s.width + cellWidth - 1) / cellWidth + // The columns at which its height fills the box, so a tall image is not cut off. + tall := s.width * previewHeight * cellHeight / (s.height * cellWidth) + return int(max(min(own, previewWidth, tall), 1)) } // previewCell returns the size of a cell in image pixels: the terminal's for kitty, and one pixel by two for half blocks. diff --git a/internal/ui/chat/model.go b/internal/ui/chat/model.go index 12d734a..df29ef1 100644 --- a/internal/ui/chat/model.go +++ b/internal/ui/chat/model.go @@ -368,9 +368,13 @@ func (p paneWidget) Draw(screen tview.Screen, area tview.Rectangle) { func (p paneWidget) Handle(msg tview.Msg, area tview.Rectangle) tview.Msg { out := p.child.Handle(msg, area) // Mouse input the pane leaves, but for a press, goes on to the other panes, for a slider dragged out of one to follow the pointer. - if mouse, ok := msg.(tview.MouseMsg); ok && area.Contains(mouse.Position()) && (out != msg || mouse.Action == tview.MouseLeftDown) { - return paneMsg{pane: p.pane, msg: out, focus: mouse.Action == tview.MouseLeftDown} + if mouse, ok := msg.(tview.MouseMsg); ok && area.Contains(mouse.Position()) { + pressed := mouse.Action == tview.MouseLeftDown + if out != msg || pressed { + return paneMsg{pane: p.pane, msg: out, focus: pressed} + } } + // Keys and paste reach only the focused pane, so what it makes of them goes to that pane through route. return out } diff --git a/internal/ui/tree/tree.go b/internal/ui/tree/tree.go index 739ba68..a322d5f 100644 --- a/internal/ui/tree/tree.go +++ b/internal/ui/tree/tree.go @@ -20,7 +20,16 @@ type ScrollMsg struct { } // New returns a boxed tree of the nodes under root in a viewport. -func New(root, current *tree.Node, scrollState viewport.ScrollState, cfg *config.Config, theme config.TreeThemeConfig, graphics bool, kbs config.TreeKeybinds, focused bool, onChange, onSelect func(*tree.Node) tview.Msg) box.Widget { +func New( + root, current *tree.Node, + scrollState viewport.ScrollState, + cfg *config.Config, + theme config.TreeThemeConfig, + graphics bool, + kbs config.TreeKeybinds, + focused bool, + onChange, onSelect func(*tree.Node) tview.Msg, +) box.Widget { set := cfg.UI.Border.NormalSet.BorderSet if focused { set = cfg.UI.Border.ActiveSet.BorderSet