PR: Fix opts.title Behaviors of _make_floating_popup_size

PR: neovim#33016 fixes Issue: neovim#33009

Background

I discovered this issue while browsing through the Neovim issue tracker.

Understanding the Problem

The issue reported unexpected behavior with opts.title in _make_floating_popup_size. My investigation revealed that while opts.title worked correctly with simple string values, it failed to handle the [string, string][] format (an array of text-highlight pairs).

After discussing with the issue author, I learned that vim.lsp.util.open_floating_preview internally uses vim.api.nvim_open_win, which supports both title formats according to the Neovim API specification. The bug was clear: _make_floating_popup_size needed to calculate width correctly for both title formats.

Implementation Journey

Initial Attempt

My first implementation was functional but had critical flaws:

if opts.title then
  local title = opts.title
  local title_length = 0
  if type(title) == 'string' then
    title_length = vim.fn.strdisplaywidth(title)
  else
    if title ~= nil then
      for i = 1, #title do
        title_length = title_length + string.len(title[i][1])
      end
    end
  end
  width = math.max(width, title_length)

Issues with this approach:

  1. Incorrect CJK handling: Using string.len() instead of vim.fn.strdisplaywidth() resulted in wrong width calculations for CJK (Chinese, Japanese, Korean) characters.
  2. Code complexity: The nested conditionals made the logic harder to follow and maintain.

Refined Solution

@luukvbaal suggested a much more elegant approach:

local title_length = 0
local chunks = type(opts.title) == 'string' and { { opts.title } } or opts.title or {}
for _, chunk in
  ipairs(chunks --[=[@as [string, string][]]=])
do
  title_length = title_length + vim.fn.strdisplaywidth(chunk[1])

Why this is better:

  • Unified handling: Normalizes both title formats (string and [string, string][]) into a consistent array structure
  • Cleaner logic: Eliminates nested conditionals with a concise ternary expression
  • Type safety: Uses Lua's type annotation syntax --[=[@as [string, string][]]=] to help with LSP type checking

This elegant solution taught me a new Lua pattern that I hadn't encountered before.

Testing

Before submitting for review, I added a unit test following the existing test patterns:

it('considers [string,string][] title when computing width', function()
  eq(
    { 17, 2 },
    exec_lua(function()
      return {
        vim.lsp.util._make_floating_popup_size(
          { 'foo', 'bar' },
          { title = { { 'A very ', 'Normal' }, { 'long title', 'Normal' } } }
        ),
      }
    end)
  )
end)

The test validates that the function correctly computes the width when given a title with multiple styled chunks.

Outcome

After pushing the changes and requesting review, @zeertzjq merged the PR successfully.

Key Takeaways

  1. Communication is crucial: Discussing with the issue author helped me understand the underlying API design and expected behavior.

  2. Learn from reviewers: @luukvbaal's suggestion introduced me to more idiomatic Lua patterns and the type annotation syntax for better code quality.

  3. Don't be intimidated: Contributing to large codebases like Neovim is more approachable than it seems. Starting with well-defined issues and learning from reviewer feedback makes the process manageable and educational.