Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions doc/markview.nvim-markdown.txt
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,7 @@ Demo {img:2}
min_width = 60,
pad_amount = 2,
pad_char = " ",
block_on_wrap = "off",

default = {
block_hl = "MarkviewCode",
Expand Down Expand Up @@ -339,6 +340,7 @@ code_blocks: block ~
---@field min_width? integer Minimum width of the code block.
---@field pad_amount? integer Number of `pad_char`s to add on the left & right side of the code block.
---@field pad_char? string Character used as padding.
---@field block_on_wrap? "off" | "on" | "adaptive" Behaviour of the `block` style under `wrap`. `"off"`(default) uses `simple`; `"on"` always keeps `block`; `"adaptive"` keeps `block` only when it fits the window.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really understand why we need literal strings as possible field values. Lua doesn't have enums, no point in trying to replicate it.

It's better to do it like older plugins using integer values instead.

Also fields with multiple possible values should span multiple lines,

---@field block_on_wrap?
---| 0 Default
---| 1 Enabled
---| 2 Automatic

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

---@field label_direction? "left" | "right"

is the precedent that justified this

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you actually look at the code or are you just copy pasting from your agent?

1 == "on", 0 == "off", that's used by other programs. People can understand that. I can't say 0 == "left", 1 == "right" since nobody uses that.

Using arbitrary strings as values tends to not stick in the long term. I would much rather not deal with spelling mistakes from end users or issues asking why only 2 values are getting checked etc.

---
---@field sign? boolean Enables signs for the code block?
---@field sign_hl? string Highlight group for the sign.
Expand All @@ -352,8 +354,10 @@ code_blocks: block ~
Creates a block around the code block.

▋  Important
▋ This is disabled if `wrap` is enabled. Or if you used `tab` inside the code
▋ block.
▋ When `wrap` is enabled this falls back to the `simple` style, unless
▋ `block_on_wrap` is set to `"on"` (always block) or `"adaptive"` (block only
▋ when it fits the window). It is also disabled if you used `tab` inside the
▋ code block.

code_blocks: simple ~

Expand Down
2 changes: 2 additions & 0 deletions lua/markview/config/markdown.lua
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,8 @@ return {
pad_amount = 2,
pad_char = " ",

block_on_wrap = "off",

default = {
block_hl = "MarkviewCode",
pad_hl = "MarkviewCode"
Expand Down
202 changes: 156 additions & 46 deletions lua/markview/renderers/markdown.lua
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,42 @@ markdown.code_block = function (buffer, item)
return line_conf;
end

--- Block width & per-line widths, computed lazily and memoized so the
--- `adaptive` overflow check and `render_block()` share a single pass.
---@type integer?
local block_width;
---@type integer[]?
local line_widths;

--- Computes `block_width` and `line_widths` once, on first use.
---@return integer block_width
---@return integer[] line_widths
local function compute_widths ()
if block_width ~= nil and line_widths ~= nil then
return block_width, line_widths;
end

---@cast config markview.config.markdown.code_blocks.block
local pad_amount = config.pad_amount or 0;
block_width = config.min_width or 60;
line_widths = {};

for l, line in ipairs(item.text) do
local final = require("markview.renderers.common.visual_text").get_visual_text(item.language, line);

if l ~= 1 and l ~= #item.text then
local w = vim.fn.strdisplaywidth(final);
table.insert(line_widths, w);

if w > (block_width - (2 * pad_amount)) then
block_width = w + (2 * pad_amount);
end
end
end

return block_width, line_widths;
end

--[[ *Basic* rendering of `code blocks`. ]]
local function render_simple()
---|fS
Expand Down Expand Up @@ -456,24 +492,10 @@ markdown.code_block = function (buffer, item)
---|fS "chunk: Calculate various widths"

local pad_amount = config.pad_amount or 0;
local block_width = config.min_width or 60;

local pad_char = config.pad_char or " ";

---@type integer[] Visual width of lines.
local line_widths = {};

for l, line in ipairs(item.text) do
local final = require("markview.renderers.common.visual_text").get_visual_text(item.language, line);

if l ~= 1 and l ~= #item.text then
table.insert(line_widths, vim.fn.strdisplaywidth(final));

if vim.fn.strdisplaywidth(final) > (block_width - (2 * pad_amount)) then
block_width = vim.fn.strdisplaywidth(final) + (2 * pad_amount);
end
end
end
---@type integer, integer[]
local block_width, line_widths = compute_widths();

local label_width = utils.virt_len({ label });
---|fE
Expand Down Expand Up @@ -705,7 +727,47 @@ markdown.code_block = function (buffer, item)
---|fE
end

if not win or config.style == "simple" or item.uses_tab or ( vim.o.wrap == true or vim.wo[win].wrap == true ) then
-- Decide between the `block` and `simple` renderers.
--
-- The `block` renderer pads each line with `inline` virtual text to reach
-- `block_width`. With `wrap` enabled that trailing padding wraps onto the
-- next screen line, breaking the block. `block_on_wrap` controls what happens
-- under `wrap`:
-- "off" (default) always fall back to `render_simple()`.
-- "on" always keep the `block` style.
-- "adaptive" keep `block` only when it fits the window; else `simple`.
local function wrap_forces_simple ()
if vim.o.wrap ~= true and vim.wo[win].wrap ~= true then
-- `wrap` is off: the block never wraps, always safe.
return false;
end

local mode = config.block_on_wrap or "off";

if mode == "on" then
return false;
elseif mode == "adaptive" then
-- Fall back to `simple` only if the rendered block would overflow the
-- window's text area (and thus wrap). `textoff` already accounts for
-- the sign, number and fold columns.
local wininfo = vim.fn.getwininfo(win)[1];
local usable = wininfo.width - wininfo.textoff;
local width = compute_widths();

-- The fence delimiter rows (```` ```lang ```` / ```` ``` ````) are
-- concealed but still counted by the wrap engine, so their raw byte
-- width is added *on top of* the `block_width` padding. Account for
-- the widest delimiter so those rows do not wrap unnoticed.
local delim_extra = math.max(#item.text[1], #item.text[#item.text]);

return (item.range.col_start + width + delim_extra) > usable;
else

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be an else if as this is now "off" | "adaptive" | String instead of the original type.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

else if what condition? This is the fall through case of neither on nor adaptive

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we are explicitly supporting 3 values, we should only match them. The project already has enough dead code that I can't refactor simply cause someone might be using the exception as the default behavior, I would much rather not have more code added to that pile.

-- "off" (or any unknown value): always fall back under `wrap`.
return true;
end
end

if not win or config.style == "simple" or item.uses_tab or wrap_forces_simple() then
render_simple();
elseif config.style == "block" then
render_block()
Expand Down Expand Up @@ -747,6 +809,55 @@ markdown.indented_code_block = function (buffer, item)
local decorations = filetypes.get("indented");
local label = { string.format(" %s%s ", decorations.icon, decorations.name), config.label_hl or decorations.icon_hl };

-- Widths, computed lazily and memoized so the `adaptive` overflow check and
-- `render_block()` share a single pass.
---@type integer?
local block_width;
---@type integer[]?
local line_widths;
---@type string[]?
local text;
---@type integer?
local pad_width;
---@type integer?
local label_width;

--- Computes the block/line widths once, on first use.
---@return integer block_width
local function compute_widths ()
if block_width ~= nil then
return block_width;
end

text = vim.api.nvim_buf_get_lines(buffer, range.row_start, range.row_end, false);

local pad_amount = config.pad_amount --[[@as integer]] or 0;
local pad_char = config.pad_char --[[@as string]] or " ";
block_width = config.min_width --[[@as integer]] or 60;
label_width = utils.virt_len({ label });
pad_width = vim.fn.strdisplaywidth(string.rep(pad_char, pad_amount));
line_widths = {};

for l, _ in ipairs(item.text) do
local final = string.sub(text[l], range.space_end);
local w = vim.fn.strdisplaywidth(final);

table.insert(line_widths, w);

if l == 1 then
if (pad_amount + w + label_width) > block_width then
block_width = pad_amount + w + label_width;
end
else
if (pad_amount + w + pad_amount) > block_width then
block_width = pad_amount + w + pad_amount;
end
end
end

return block_width;
end

--[[ *Basic* rendering of `code blocks`. ]]
local function render_simple()
---|fS
Expand Down Expand Up @@ -784,37 +895,15 @@ markdown.indented_code_block = function (buffer, item)

---|fS "chunk: Calculate various widths"

local text = vim.api.nvim_buf_get_lines(buffer, range.row_start, range.row_end, false);

local pad_amount = config.pad_amount --[[@as integer]] or 0;
local block_width = config.min_width --[[@as integer]] or 60;

local pad_char = config.pad_char --[[@as string]] or " ";
local label_width = utils.virt_len({ label });

local pad_width = vim.fn.strdisplaywidth(
string.rep(pad_char, pad_amount)
);

---@type integer[] Visual width of lines.
local line_widths = {};

for l, _ in ipairs(item.text) do
local final = string.sub(text[l], range.space_end);
local w = vim.fn.strdisplaywidth(final)

table.insert(line_widths, w);

if l == 1 then
if (pad_amount + w + label_width) > block_width then
block_width = pad_amount + w + label_width;
end
else
if (pad_amount + w + pad_amount) > block_width then
block_width = pad_amount + w + pad_amount;
end
end
end
compute_widths();
---@cast text string[]
---@cast line_widths integer[]
---@cast block_width integer
---@cast pad_width integer
---@cast label_width integer

---|fE

Expand Down Expand Up @@ -897,7 +986,28 @@ markdown.indented_code_block = function (buffer, item)
---@type integer Window containing `buffer`.
local win = utils.buf_getwin(buffer);

if not win or config.style == "simple" or ( vim.o.wrap == true or vim.wo[win].wrap == true ) then
-- See `markdown.code_block` for the meaning of `block_on_wrap`.
local function wrap_forces_simple ()
if vim.o.wrap ~= true and vim.wo[win].wrap ~= true then
return false;
end

local mode = config.block_on_wrap or "off";

if mode == "on" then
return false;
elseif mode == "adaptive" then
local wininfo = vim.fn.getwininfo(win)[1];

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getwininfo() is too performance intensive. It is not suitable for large buffers.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First, it's in the adaptive mode and only opt-in.
Second, i use markview daily in 10k+ LOC buffers and it works well.

That said. what alternative do you suggest?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no alternatives(at the moment) which is why this option wasn't provided. You can't correctly get a line's final width without having performance cost.

Even if you are using getwininfo() once every code block you are still constantly calling the API. And it tends to not work on text outside of the visible window area, so it leaves edge cases.


Also, LOC isn't what you should be measuring, the complexity & depth of the AST is what you should be looking at when doing performance optimizations.

local usable = wininfo.width - wininfo.textoff;
local width = compute_widths();

return (range.col_start + width) > usable;
else
return true;
end
end

if not win or config.style == "simple" or wrap_forces_simple() then
render_simple();
elseif config.style == "block" then
render_block()
Expand Down
10 changes: 10 additions & 0 deletions lua/markview/types/renderers/markdown.lua
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,15 @@
---@field [string] markview.config.markdown.code_blocks.opts


--- Controls whether the `block` style is kept when `wrap` is enabled.
--- - `"off"` Fall back to the `simple` style (default).
--- - `"on"` Always keep the `block` style.
--- - `"adaptive"` Keep the `block` style only when the block fits the window; else `simple`.
---@alias markview.config.markdown.code_blocks.block_on_wrap
---| "off"
---| "on"
---| "adaptive"
---
---@class markview.config.markdown.code_blocks.block
---
---@field enable boolean Enable rendering of code blocks.
Expand All @@ -86,6 +95,7 @@
---@field min_width? integer Minimum width of the code block.
---@field pad_amount? integer Number of `pad_char`s to add on the left & right side of the code block.
---@field pad_char? string Character used as padding.
---@field block_on_wrap? markview.config.markdown.code_blocks.block_on_wrap Controls the `block` style under `wrap`. `"off"`(default) falls back to the `simple` style; `"on"` always keeps the `block` style; `"adaptive"` keeps `block` only when it fits the window (else `simple`).
---
---@field sign? boolean Enables signs for the code block?
---@field sign_hl? string Highlight group for the sign.
Expand Down
Loading