refactor: clean up renderer/formatter

Now that we have one code path, I could remove some wrapper functions.

Also cleaned up some times / variable names to be more consistent.

Author: cameronr

lua/opencode/ui/formatter.lua

@@ -242,25 +242,28 @@ function M._format_error(output, message)
   M._format_callout(output, 'ERROR', vim.inspect(message.error))
 end
 
----@param output Output Output object to write to
----@param message MessageInfo
+---@param message OpencodeMessage
 ---@param msg_idx number Message index in the session
-function M._format_message_header(output, message, msg_idx)
-  local role = message.role or 'unknown'
-  local icon = message.role == 'user' and icons.get('header_user') or icons.get('header_assistant')
+---@return Output
+function M.format_message_header(message, msg_idx)
+  local output = Output.new()
+
+  output:add_lines(M.separator)
+  local role = message.info.role or 'unknown'
+  local icon = message.info.role == 'user' and icons.get('header_user') or icons.get('header_assistant')
 
-  local time = message.time and message.time.created or nil
+  local time = message.info.time and message.info.time.created or nil
   local time_text = (time and ' (' .. util.time_ago(time) .. ')' or '')
   local role_hl = 'OpencodeMessageRole' .. role:sub(1, 1):upper() .. role:sub(2)
-  local model_text = message.modelID and ' ' .. message.modelID or ''
-  local debug_text = config.debug and ' [' .. message.id .. ']' or ''
+  local model_text = message.info.modelID and ' ' .. message.info.modelID or ''
+  local debug_text = config.debug and ' [' .. message.info.id .. ']' or ''
 
   output:add_empty_line()
   output:add_metadata({ msg_idx = msg_idx, part_idx = 1, role = role, type = 'header' })
 
   local display_name
   if role == 'assistant' then
-    local mode = message.mode
+    local mode = message.info.mode
     if mode and mode ~= '' then
       display_name = mode:upper()
     else
@@ -291,6 +294,7 @@ function M._format_message_header(output, message, msg_idx)
   })
 
   output:add_line('')
+  return output
 end
 
 ---@param output Output Output object to write to
@@ -660,23 +664,28 @@ function M._add_vertical_border(output, start_line, end_line, hl_group, win_col)
   end
 end
 
----Internal function that formats a message part into an existing output object
----@param output Output Output object to write to
----@param part MessagePart
----@param message_info {msg_idx: number, part_idx: number, role: string, message: table}
-function M._format_part(output, part, message_info)
+---Formats a single message part and returns the resulting output object
+---@param part MessagePart The part to format
+---@param role 'user'|'assistant'|'system' The role, user or assistant, that created this part
+---@param msg_idx integer The index of the message in state.messages
+---@param part_idx integer The index of the part in state.messages[msg_idx].parts
+---@return Output
+function M.format_part(part, role, msg_idx, part_idx)
+  local output = Output.new()
+
+  -- FIXME: do we need metadata? it looks like maybe only for snapshots?
   local metadata = {
-    msg_idx = message_info.msg_idx,
-    part_idx = message_info.part_idx,
-    role = message_info.role,
+    msg_idx = msg_idx,
+    part_idx = part_idx,
+    role = role,
     type = part.type,
     snapshot = part.snapshot,
   }
   output:add_metadata(metadata)
 
   local content_added = false
 
-  if message_info.role == 'user' then
+  if role == 'user' then
     if part.type == 'text' and part.text then
       if part.synthetic == true then
         M._format_selection_context(output, part)
@@ -692,7 +701,7 @@ function M._format_part(output, part, message_info)
       M._add_vertical_border(output, file_line - 1, file_line, 'OpencodeMessageRoleUser', -3)
       content_added = true
     end
-  elseif message_info.role == 'assistant' then
+  elseif role == 'assistant' then
     if part.type == 'text' and part.text then
       M._format_assistant_message(output, vim.trim(part.text))
       content_added = true
@@ -708,26 +717,6 @@ function M._format_part(output, part, message_info)
   if content_added then
     output:add_empty_line()
   end
-end
-
----Public function that formats a single message part and returns a new output object
----@param part MessagePart
----@param message_info {msg_idx: number, part_idx: number, role: string, message: table}
----@return Output
-function M.format_part_single(part, message_info)
-  local output = Output.new()
-  M._format_part(output, part, message_info)
-  return output
-end
-
----@param message OpencodeMessage
----@param msg_idx number
----@return Output
-function M.format_message_header_single(message, msg_idx)
-  local output = Output.new()
-
-  output:add_lines(M.separator)
-  M._format_message_header(output, message.info, msg_idx)
 
   return output
 end

lua/opencode/ui/renderer.lua

@@ -374,15 +374,10 @@ end
 
 ---Event handler for message.updated events
 ---Creates new message or updates existing message info
----@param properties {info: MessageInfo} Event properties
-function M.on_message_updated(properties)
-  if not properties or not properties.info then
-    return
-  end
-
+---@param message {info: MessageInfo} Event properties
+function M.on_message_updated(message)
   ---@type OpencodeMessage
-  local message = properties
-  if not message.info.id or not message.info.sessionID then
+  if not message or not message.info or not message.info.id or not message.info.sessionID then
     return
   end
 
@@ -400,7 +395,7 @@ function M.on_message_updated(properties)
     found_idx = #state.messages
     M._message_map:add_message(message.info.id, found_idx)
 
-    local header_data = formatter.format_message_header_single(message, found_idx)
+    local header_data = formatter.format_message_header(message, found_idx)
     M._write_formatted_data(header_data)
 
     state.current_message = message
@@ -433,22 +428,21 @@ function M.on_part_updated(properties)
     return
   end
 
-  local msg_wrapper, msg_idx = M._message_map:get_message_by_id(part.messageID, state.messages)
+  local message, msg_idx = M._message_map:get_message_by_id(part.messageID, state.messages)
 
-  if not msg_wrapper or not msg_idx then
+  if not message or not msg_idx then
     vim.notify('Could not find message for part: ' .. vim.inspect(part), vim.log.levels.WARN)
     return
   end
 
-  local message = msg_wrapper.info
-  msg_wrapper.parts = msg_wrapper.parts or {}
+  message.parts = message.parts or {}
 
   local is_new_part = not M._message_map:has_part(part.id)
   local part_idx
 
   if is_new_part then
-    table.insert(msg_wrapper.parts, part)
-    part_idx = #msg_wrapper.parts
+    table.insert(message.parts, part)
+    part_idx = #message.parts
     M._message_map:add_part(part.id, msg_idx, part_idx, part.callID)
   else
     part_idx = M._message_map:update_part(part.id, part, state.messages)
@@ -473,17 +467,7 @@ function M.on_part_updated(properties)
     }
   end
 
-  local ok, formatted = pcall(formatter.format_part_single, part, {
-    msg_idx = msg_idx,
-    part_idx = part_idx,
-    role = message.role,
-    message = msg_wrapper,
-  })
-
-  if not ok then
-    vim.notify('format_part_single error: ' .. tostring(formatted), vim.log.levels.ERROR)
-    return
-  end
+  local formatted = formatter.format_part(part, message.info.role, msg_idx, part_idx)
 
   if is_new_part then
     M._insert_part_to_buffer(part.id, formatted)
@@ -535,8 +519,8 @@ function M.on_message_removed(properties)
     return
   end
 
-  local msg_wrapper = state.messages[message_idx]
-  for _, part in ipairs(msg_wrapper.parts or {}) do
+  local message = state.messages[message_idx]
+  for _, part in ipairs(message.parts or {}) do
     if part.id then
       M._remove_part_from_buffer(part.id)
     end
@@ -572,28 +556,18 @@ end
 
 ---Event handler for permission.updated events
 ---Re-renders part that requires permission
----@param properties OpencodePermission Event properties
-function M.on_permission_updated(properties)
-  if not properties then
+---@param permission OpencodePermission Event properties
+function M.on_permission_updated(permission)
+  if not permission or not permission.messageID or not permission.callID then
     return
   end
 
-  local permission = properties
-  if not permission.messageID or not permission.callID then
-    return
-  end
-
-  local prev_permission = state.current_permission
-  state.current_permission = nil
-
-  if prev_permission and prev_permission.id ~= permission.id then
+  if state.current_permission and state.current_permission.id ~= permission.id then
     -- we got a permission request while we had an existing one?
-    vim.notify('Two pending permissions? existing: ' .. prev_permission.id .. ' new: ' .. permission.id)
-    -- rerender previous part to remove old permission
-    local prev_perm_part_id = M._find_part_by_call_id(prev_permission.callID)
-    if prev_perm_part_id then
-      M._rerender_part(prev_perm_part_id)
-    end
+    vim.notify('Two pending permissions? existing: ' .. state.current_permission.id .. ' new: ' .. permission.id)
+
+    -- This will rerender the part with the old permission
+    M.on_permission_replied({})
   end
 
   state.current_permission = permission
@@ -647,24 +621,13 @@ function M._rerender_part(part_id)
     return
   end
 
-  local part, msg_wrapper, msg_idx, part_idx = M._message_map:get_part_by_id(part_id, state.messages)
+  local part, message, msg_idx, part_idx = M._message_map:get_part_by_id(part_id, state.messages)
 
-  if not part or not msg_wrapper then
+  if not part or not message or not msg_idx or not part_idx then
     return
   end
 
-  local message_with_parts = vim.tbl_extend('force', msg_wrapper.info, { parts = msg_wrapper.parts })
-  local ok, formatted = pcall(formatter.format_part_single, part, {
-    msg_idx = msg_idx or 1,
-    part_idx = part_idx or 1,
-    role = msg_wrapper.info.role,
-    message = message_with_parts,
-  })
-
-  if not ok then
-    vim.notify('format_part_single error: ' .. tostring(formatted), vim.log.levels.ERROR)
-    return
-  end
+  local formatted = formatter.format_part(part, message.info.role, msg_idx, part_idx)
 
   M._replace_part_in_buffer(part_id, formatted)
 end