Skip to content

Commit

Permalink
fix(render): handle large messages when calculating line width
Browse files Browse the repository at this point in the history
Some misbehaving plugin code was sending very large messages to
`vim.notify()`, which would trigger an exception when calculating the
max line width for rendering:

```
[...]/notify/render/default.lua:6: too many results to unpack
```

To fix this, I updated the implementation to calculate the max line
width in-place. This ultimately ends up being much more efficient because
it is not creating a new table with `vim.tbl_map()`.

Here's some benchmark data:
https://gist.github.com/flrgh/349aecd8ca35d20cf0526ec8b218657c

In the large input benchmarks, the new function is ~2x faster. For small
inputs (which are probably most common for `vim.notify()`), the speedup
is even larger at 3-4x.

Other changes:

  1. I noticed the same snippet appeared in multiple render modules, so
     I factored it out into `notify.util.max_line_width()` and added a
     unit test
  2. I updated the calculation to use `vim.api.nvim_strwidth()` when
     available (see #247).
  • Loading branch information
flrgh committed Jun 20, 2024
1 parent d333b6f commit 7b4075b
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 6 deletions.
6 changes: 3 additions & 3 deletions lua/notify/render/default.lua
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
local api = vim.api
local base = require("notify.render.base")
local util = require("notify.util")

return function(bufnr, notif, highlights, config)
local left_icon = notif.icon .. " "
local max_message_width = math.max(math.max(unpack(vim.tbl_map(function(line)
return vim.fn.strchars(line)
end, notif.message))))
local max_message_width = util.max_line_width(notif.message)

local right_title = notif.title[2]
local left_title = notif.title[1]
local title_accum = vim.str_utfindex(left_icon)
Expand Down
6 changes: 3 additions & 3 deletions lua/notify/render/simple.lua
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
local api = vim.api
local base = require("notify.render.base")
local util = require("notify.util")

return function(bufnr, notif, highlights, config)
local max_message_width = math.max(math.max(unpack(vim.tbl_map(function(line)
return vim.fn.strchars(line)
end, notif.message))))
local max_message_width = util.max_line_width(notif.message)

local title = notif.title[1]
local title_accum = vim.str_utfindex(title)

Expand Down
17 changes: 17 additions & 0 deletions lua/notify/util/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ local M = {}

local min, max, floor = math.min, math.max, math.floor
local rshift, lshift, band, bor = bit.rshift, bit.lshift, bit.band, bit.bor
local strwidth = vim.api.nvim_strwidth or vim.fn.strchars

function M.is_callable(obj)
return type(obj) == "function" or (type(obj) == "table" and obj.__call)
end
Expand Down Expand Up @@ -117,4 +119,19 @@ function M.highlight(name, fields)
end
end

--- Calculate the max render width of a message
---@param msg string[]|nil
---@return integer
function M.max_line_width(msg)
local width = 0

if msg then
for i = 1, #msg do
width = max(width, strwidth(msg[i]))
end
end

return width
end

return M
14 changes: 14 additions & 0 deletions tests/unit/init_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -157,4 +157,18 @@ describe("checking public interface", function()
a.util.scheduler()
assert(vim.api.nvim_win_is_valid(win))
end)

describe("util", function()
local util = require("notify.util")

describe("max_line_width()", function()
it("returns the maximal width of a table of lines", function()
assert.equals(5, util.max_line_width({ "12", "12345", "123" }))
end)

it("returns 0 for nil input", function()
assert.equals(0, util.max_line_width())
end)
end)
end)
end)

0 comments on commit 7b4075b

Please sign in to comment.