From f2263f9f123dafe2adc406e1ab553e4205860a34 Mon Sep 17 00:00:00 2001 From: David Herman Date: Tue, 9 Jun 2026 11:44:01 +0200 Subject: [PATCH] fix(picker): align datetime column consistently in time-based pickers The time column in session and timeline pickers had two alignment issues causing the datetime to render partially or fully off-screen, especially when a preview pane was active. Per-item variable width: each row computed its own time column width from format_time(timestamp), producing different widths for same-day (e.g. '10:35') vs older entries (e.g. '09 Jun 2024 10:35'). Fixed by using format_time(0) to derive the maximum column width from the longest possible output (different-year format). Shorter time strings are right-padded within the fixed column. Separator double-counting: the +1 in time_width and debug_width was intended for the separator between parts, but to_string() and to_formatted_text() already add their own separators. This caused rows to exceed item_width by 1-2 chars, pushing the time column past the visible pane edge. Fixed by accounting for separators explicitly. Unreliable fzf width_callback: the start:+transform callback read FZF_PREVIEW_COLUMNS to compute list pane width, but this env var could be 0 or absent at fzf start time. When missing, format_width fell back to the full window width, formatting items much wider than the visible list pane. Removed this mechanism in favor of the M.pick-level format_width which already accounts for the preview split ratio. --- lua/opencode/ui/base_picker.lua | 57 +++++---- lua/opencode/ui/session_picker.lua | 13 +- lua/opencode/ui/timeline_picker.lua | 14 ++- tests/unit/base_picker_spec.lua | 188 +++++++++++++++++++++++++--- 4 files changed, 223 insertions(+), 49 deletions(-) diff --git a/lua/opencode/ui/base_picker.lua b/lua/opencode/ui/base_picker.lua index 94d21918..d0c3a8a3 100644 --- a/lua/opencode/ui/base_picker.lua +++ b/lua/opencode/ui/base_picker.lua @@ -315,9 +315,9 @@ end local function fzf_ui(opts) local fzf_lua = require('fzf-lua') - local function finder(fzf_cb, width) + local function finder(fzf_cb) for idx, item in ipairs(opts.items) do - local line_str = opts.format_fn(item, width):to_string() + local line_str = opts.format_fn(item):to_string() -- Prepend index with SOH delimiter for reliable matching local indexed_line = tostring(idx) .. '\x01' .. line_str @@ -352,16 +352,6 @@ local function fzf_ui(opts) end local has_custom_preview = opts.preview == 'custom' and opts.preview_fn ~= nil - local format_width - - -- defer item processing until preview if using custom preview_fn - -- so we can pass the width of the fzf window to the format_fn for optimal formatting - local width_callback = has_custom_preview - and function(_, _, _, ctx) - format_width = (ctx.env.FZF_COLUMNS or vim.o.columns) - (ctx.env.FZF_PREVIEW_COLUMNS or 0) - format_width = math.min(format_width, vim.o.columns - 8) - end - or nil ---@return table local function create_fzf_config() @@ -370,9 +360,6 @@ local function fzf_ui(opts) end) return { - fzf_cli_args = width_callback and ('--bind=' .. require('fzf-lua.libuv').shellescape( - 'start:+transform:' .. require('fzf-lua.shell').stringify_data(width_callback, opts) - )) or nil, winopts = opts.width and { width = opts.width + 8, -- extra space for fzf UI } or nil, @@ -528,12 +515,7 @@ local function fzf_ui(opts) fzf_config.actions = actions_config fzf_lua.fzf_exec(function(fzf_cb) - if width_callback then - vim.wait(1000, function() - return format_width - end) - end - finder(fzf_cb, format_width) + finder(fzf_cb) end, fzf_config) end @@ -813,18 +795,41 @@ function M.create_picker_item(parts) return item end +---Compute the maximum formatted time width across a list of items. +---@param items any[] The list of items +---@param get_time_fn fun(item: any): number? Extracts a timestamp from an item +---@return number max_width The width of the longest format_time result (0 if no valid timestamps) +function M.max_time_width(items, get_time_fn) + local max_w = 0 + for _, item in ipairs(items) do + local t = get_time_fn(item) + if t and type(t) == 'number' then + local w = #util.format_time(t) + if w > max_w then + max_w = w + end + end + end + return max_w +end + ---Helper function to create a simple picker item with content, time, and debug text ---This is a convenience wrapper around create_picker_item for common use cases ---@param text string Main content text ---@param time? number Optional time to format ---@param debug_text? string Optional debug text to append ---@param width? number Optional width override +---@param max_time_width? number Optional pre-computed max time column width from max_time_width() ---@return PickerItem -function M.create_time_picker_item(text, time, debug_text, width) - local time_width = time and #util.format_time(time) + 1 or 0 - local debug_width = config.debug.show_ids and debug_text and #debug_text + 1 or 0 +function M.create_time_picker_item(text, time, debug_text, width, max_time_width) + local time_width = time and (max_time_width or #util.format_time(0)) or 0 + local has_debug = config.debug.show_ids and debug_text + local debug_width = has_debug and #debug_text or 0 local item_width = width or vim.api.nvim_win_get_width(0) - local text_width = item_width - (debug_width + time_width) + -- Each extra part adds a 1-char separator in to_string()/to_formatted_text(), + -- so subtract those from the text budget. + local separator_count = (time_width > 0 and 1 or 0) + (debug_width > 0 and 1 or 0) + local text_width = item_width - time_width - debug_width - separator_count local parts = { { @@ -839,7 +844,7 @@ function M.create_time_picker_item(text, time, debug_text, width) }) end - if config.debug.show_ids and debug_text then + if has_debug then table.insert(parts, { text = debug_text, highlight = 'OpencodeDebugText', diff --git a/lua/opencode/ui/session_picker.lua b/lua/opencode/ui/session_picker.lua index 654af1a9..98e4b84b 100644 --- a/lua/opencode/ui/session_picker.lua +++ b/lua/opencode/ui/session_picker.lua @@ -30,8 +30,9 @@ end ---Format session parts for session picker ---@param session Session|GlobalSession object ---@param width? integer +---@param max_tw? number Pre-computed max time column width ---@return PickerItem -function format_session_item(session, width) +local function format_session_item(session, width, max_tw) local project = (session --[[@as GlobalSession]]).project local title = session.title or 'N/A' if project then @@ -39,7 +40,7 @@ function format_session_item(session, width) title = title .. ' [' .. label .. ']' end local updated_time = (session.time and session.time.updated) or 'N/A' - return base_picker.create_time_picker_item(title, updated_time, nil, width) + return base_picker.create_time_picker_item(title, updated_time, nil, width, max_tw) end --- Normalize message order to oldest-first (chronological) @@ -341,9 +342,15 @@ function M.pick(sessions, callback, opts) -- Preview state for race condition protection local preview_seq = 0 + local max_tw = base_picker.max_time_width(sessions, function(s) + return s.time and s.time.updated + end) + return base_picker.pick({ items = sessions, - format_fn = format_session_item, + format_fn = function(session, width) + return format_session_item(session, width, max_tw) + end, actions = actions, callback = callback, title = (opts and opts.scope == 'global') and 'Select A Session (all projects)' or 'Select A Session', diff --git a/lua/opencode/ui/timeline_picker.lua b/lua/opencode/ui/timeline_picker.lua index e55fe39b..b26d4d84 100644 --- a/lua/opencode/ui/timeline_picker.lua +++ b/lua/opencode/ui/timeline_picker.lua @@ -5,13 +5,15 @@ local base_picker = require('opencode.ui.base_picker') ---Format message parts for timeline picker ---@param msg OpencodeMessage Message object +---@param width? number +---@param max_tw? number Pre-computed max time column width ---@return PickerItem -function format_message_item(msg, width) +local function format_message_item(msg, width, max_tw) local preview = msg.parts and msg.parts[1] and msg.parts[1].text or '' local debug_text = 'ID: ' .. (msg.info.id or 'N/A') - return base_picker.create_time_picker_item(vim.trim(preview), msg.info.time.created, debug_text, width) + return base_picker.create_time_picker_item(vim.trim(preview), msg.info.time.created, debug_text, width, max_tw) end function M.pick(messages, callback) @@ -35,9 +37,15 @@ function M.pick(messages, callback) }, } + local max_tw = base_picker.max_time_width(messages, function(msg) + return msg.info and msg.info.time and msg.info.time.created + end) + return base_picker.pick({ items = messages, - format_fn = format_message_item, + format_fn = function(msg, width) + return format_message_item(msg, width, max_tw) + end, actions = actions, callback = callback, title = 'Timeline', diff --git a/tests/unit/base_picker_spec.lua b/tests/unit/base_picker_spec.lua index 61fdebfa..3182328c 100644 --- a/tests/unit/base_picker_spec.lua +++ b/tests/unit/base_picker_spec.lua @@ -252,8 +252,6 @@ describe('opencode.ui.base_picker fzf-lua preview', function() ['opencode.ui.picker'] = package.loaded['opencode.ui.picker'], ['opencode.ui.base_picker'] = package.loaded['opencode.ui.base_picker'], ['fzf-lua'] = package.loaded['fzf-lua'], - ['fzf-lua.libuv'] = package.loaded['fzf-lua.libuv'], - ['fzf-lua.shell'] = package.loaded['fzf-lua.shell'], ['fzf-lua.previewer.builtin'] = package.loaded['fzf-lua.previewer.builtin'], } @@ -295,7 +293,6 @@ describe('opencode.ui.base_picker fzf-lua preview', function() captured_fzf_finder = nil captured_fzf_opts = nil - captured_transform = nil package.loaded['fzf-lua'] = { fzf_exec = function(finder, opts) @@ -305,18 +302,6 @@ describe('opencode.ui.base_picker fzf-lua preview', function() } next_preview_buf = nil - package.loaded['fzf-lua.libuv'] = { - shellescape = function(s) - return s - end, - } - - package.loaded['fzf-lua.shell'] = { - stringify_data = function(fn) - captured_transform = fn - return '' - end, - } package.loaded['fzf-lua.previewer.builtin'] = { buffer_or_file = { @@ -429,10 +414,179 @@ describe('opencode.ui.base_picker fzf-lua preview', function() end end) - captured_transform(nil, nil, nil, { env = { FZF_COLUMNS = 31 } }) - + -- picker_width=80 with preview: window=80+8=88, list pane=floor(88*0.4)-4=31 assert.equal(31, observed_width) assert.are.same({ '1\001session' }, emitted_lines) assert.equal(88, captured_fzf_opts.winopts.width) end) end) + +describe('opencode.ui.base_picker create_time_picker_item alignment', function() + local base_picker + local original_schedule + local saved_modules + local mock_config + + before_each(function() + original_schedule = vim.schedule + vim.schedule = function(fn) + fn() + end + + saved_modules = { + ['opencode.config'] = package.loaded['opencode.config'], + ['opencode.util'] = package.loaded['opencode.util'], + ['opencode.promise'] = package.loaded['opencode.promise'], + ['opencode.ui.picker'] = package.loaded['opencode.ui.picker'], + ['opencode.ui.base_picker'] = package.loaded['opencode.ui.base_picker'], + } + + mock_config = { + ui = { + picker_width = 80, + output = { time_format = nil }, + }, + debug = { show_ids = false }, + } + package.loaded['opencode.config'] = mock_config + + -- Provide a format_time that mimics real behavior: variable-width output + -- depending on how old the timestamp is relative to "now". + package.loaded['opencode.util'] = { + format_time = function(timestamp) + if not timestamp then + return '' + end + local now = os.time() + local same_day = os.date('%Y-%m-%d') == os.date('%Y-%m-%d', timestamp) + local same_year = os.date('%Y') == os.date('%Y', timestamp) + local time_part = os.date('%H:%M', timestamp) + if same_day then + return time_part -- e.g. "10:35" + elseif same_year then + return os.date('%d %b', timestamp) .. ' ' .. time_part -- e.g. "09 Jun 10:35" + else + return os.date('%d %b %Y', timestamp) .. ' ' .. time_part -- e.g. "09 Jun 2024 10:35" + end + end, + } + + package.loaded['opencode.promise'] = { + wrap = function(value) + return { + and_then = function(_, cb) + cb(value) + end, + } + end, + } + + package.loaded['opencode.ui.picker'] = { + get_best_picker = function() + return 'snacks' + end, + } + + package.loaded['opencode.ui.base_picker'] = nil + base_picker = require('opencode.ui.base_picker') + end) + + after_each(function() + vim.schedule = original_schedule + for module_name, module_value in pairs(saved_modules) do + package.loaded[module_name] = module_value + end + end) + + it('produces identical total width for timestamps of different ages', function() + local now = os.time() + local same_day_ts = now - 3600 -- 1 hour ago + local same_year_ts = now - 86400 * 60 -- ~2 months ago + local diff_year_ts = 0 -- epoch (1970) + + local width = 80 + local format_time = package.loaded['opencode.util'].format_time + local max_tw = math.max(#format_time(same_day_ts), #format_time(same_year_ts), #format_time(diff_year_ts)) + + local item_today = base_picker.create_time_picker_item('Session A', same_day_ts, nil, width, max_tw) + local item_month = base_picker.create_time_picker_item('Session B', same_year_ts, nil, width, max_tw) + local item_old = base_picker.create_time_picker_item('Session C', diff_year_ts, nil, width, max_tw) + + local str_today = item_today:to_string() + local str_month = item_month:to_string() + local str_old = item_old:to_string() + + assert.equal(#str_today, #str_month, 'same-day and same-year items should have equal width') + assert.equal(#str_today, #str_old, 'same-day and different-year items should have equal width') + end) + + it('to_string width matches the requested item width exactly', function() + local width = 60 + + local item = base_picker.create_time_picker_item('Session Title', 0, nil, width) + local str = item:to_string() + + assert.equal(width, #str, 'to_string() output should be exactly item_width chars') + end) + + it('right-aligns time within a fixed-width column', function() + local now = os.time() + local same_day_ts = now - 3600 + + local width = 80 + local max_tw = #package.loaded['opencode.util'].format_time(same_day_ts) + local item = base_picker.create_time_picker_item('Title', same_day_ts, nil, width, max_tw) + local str = item:to_string() + + local time_str = package.loaded['opencode.util'].format_time(same_day_ts) + assert.is_truthy(str:match(time_str .. '$'), 'time string should be at the right edge') + end) + + it('max_time_width returns the width of the longest formatted timestamp', function() + local now = os.time() + local items = { + { time = now - 3600 }, -- same-day + { time = now - 86400 * 60 }, -- same-year + { time = 0 }, -- different year (longest) + } + + local max_tw = base_picker.max_time_width(items, function(item) + return item.time + end) + + local format_time = package.loaded['opencode.util'].format_time + local expected = math.max(#format_time(items[1].time), #format_time(items[2].time), #format_time(items[3].time)) + assert.equal(expected, max_tw) + end) + + it('max_time_width returns 0 for empty items', function() + local max_tw = base_picker.max_time_width({}, function(item) + return item.time + end) + assert.equal(0, max_tw) + end) + + it('max_time_width returns 0 when no items have timestamps', function() + local items = { { name = 'a' }, { name = 'b' } } + local max_tw = base_picker.max_time_width(items, function(item) + return item.time -- nil + end) + assert.equal(0, max_tw) + end) + + it('uses only same-day width when all timestamps are from today', function() + local now = os.time() + local items = { + { time = now - 60 }, + { time = now - 3600 }, + } + + local max_tw = base_picker.max_time_width(items, function(item) + return item.time + end) + + local format_time = package.loaded['opencode.util'].format_time + -- All same-day, so max_tw should equal the short time width + assert.equal(#format_time(now - 60), max_tw) + end) +end)