Skip to content

Commit 51160f1

Browse files
flrghrcarriga
andauthored
fix(render): handle large messages when calculating line width (#283)
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). Co-authored-by: Rónán Carrigan <rcarriga@tcd.ie>
1 parent 85b90b6 commit 51160f1

File tree

4 files changed

+35
-6
lines changed

4 files changed

+35
-6
lines changed

lua/notify/render/default.lua

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
local api = vim.api
22
local base = require("notify.render.base")
3+
local util = require("notify.util")
34

45
return function(bufnr, notif, highlights, config)
56
local left_icon = notif.icon == "" and "" or notif.icon .. " "
6-
local max_message_width = math.max(unpack(vim.tbl_map(function(line)
7-
return vim.api.nvim_strwidth(line)
8-
end, notif.message)))
7+
local max_message_width = util.max_line_width(notif.message)
8+
99
local right_title = notif.title[2]
1010
local left_title = notif.title[1]
1111
local title_accum = vim.api.nvim_strwidth(left_icon)

lua/notify/render/simple.lua

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
local api = vim.api
22
local base = require("notify.render.base")
3+
local util = require("notify.util")
34

45
return function(bufnr, notif, highlights, config)
5-
local max_message_width = math.max(unpack(vim.tbl_map(function(line)
6-
return vim.api.nvim_strwidth(line)
7-
end, notif.message)))
6+
local max_message_width = util.max_line_width(notif.message)
7+
88
local title = notif.title[1]
99
local title_accum = vim.api.nvim_strwidth(title)
1010

lua/notify/util/init.lua

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ local M = {}
22

33
local min, max, floor = math.min, math.max, math.floor
44
local rshift, lshift, band, bor = bit.rshift, bit.lshift, bit.band, bit.bor
5+
local strwidth = vim.api.nvim_strwidth or vim.fn.strchars
6+
57
function M.is_callable(obj)
68
return type(obj) == "function" or (type(obj) == "table" and obj.__call)
79
end
@@ -117,4 +119,19 @@ function M.highlight(name, fields)
117119
end
118120
end
119121

122+
--- Calculate the max render width of a message
123+
---@param msg string[]|nil
124+
---@return integer
125+
function M.max_line_width(msg)
126+
local width = 0
127+
128+
if msg then
129+
for i = 1, #msg do
130+
width = max(width, strwidth(msg[i]))
131+
end
132+
end
133+
134+
return width
135+
end
136+
120137
return M

tests/unit/init_spec.lua

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,18 @@ describe("checking public interface", function()
181181
assert(vim.api.nvim_win_is_valid(win))
182182
end)
183183

184+
describe("util", function()
185+
local util = require("notify.util")
186+
187+
describe("max_line_width()", function()
188+
it("returns the maximal width of a table of lines", function()
189+
assert.equals(5, util.max_line_width({ "12", "12345", "123" }))
190+
end)
191+
192+
it("returns 0 for nil input", function()
193+
assert.equals(0, util.max_line_width())
194+
end)
195+
184196
describe("notification width", function()
185197
a.it("handles multibyte characters correctly", function()
186198
local instance = notify.instance({

0 commit comments

Comments
 (0)