Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(render): add support for concealed characters (#153) #156

Merged
merged 14 commits into from
Sep 20, 2023

Conversation

milisims
Copy link
Contributor

@milisims milisims commented Sep 1, 2023

For testing, a simple example would be to add the following to vim.fn.stdpath('config') .. '/after/queries/lua/highlights.scm'

; extends
(function_declaration "function" @constant (#set! conceal "𝑓"))

@milisims milisims changed the title feat: add support for concealed characters (#153) feat(render): add support for concealed characters (#153) Sep 1, 2023
@kevinhwang91
Copy link
Owner

nvim-ufo should work under 0.7.2 version for neovim which is a stable version for ubuntu 22 lts, so don't use a function program API;
Don't change the code for "better to read", the captureVirtText function is inside decoration_provider, and shouldn't use the core API.

I suggest you just change the code about concealed extmarks. Of course, you can also capture the concealed in synconcealed() if you are interested in the old highlight group system in vim.

@kevinhwang91 kevinhwang91 self-requested a review September 2, 2023 16:12
@milisims
Copy link
Contributor Author

milisims commented Sep 2, 2023

nvim-ufo should work under 0.7.2 version for neovim which is a stable version for ubuntu 22 lts, so

  • don't use a function program API
  • Don't change the code for "better to read" (I'm not sure where you think I did this exactly)
  • the captureVirtText function is inside decoration_provider, and shouldn't use the core API.

I suggest you just change the code about concealed extmarks. Of course, you can also capture the concealed in synconcealed() if you are interested in the old highlight group system in vim.

I'll add synconcealed() support after a quick review:

I originally attempted to simply add an insertConcealed, but because conceal and inlays both change the length of the text trying to track the col and endCol with an offset was a nightmare. I had to change/remove fillSlots and insertInlay. Doing them at the same time made it possible for me to implement.

How does it look now? (before I look into adding synconcealed())

@kevinhwang91
Copy link
Owner

Before checking the conceal, the 'conceallevel' option should be more than zero.

for _, m in ipairs(extMarks) do
local hlGroup, conceal = m[5], m[7]
-- next({}) ➜ nil
if conceal or (hlGroup and next(hlGroups[hlGroup])) then
Copy link
Owner

Choose a reason for hiding this comment

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

The origin code checks foreground attribute because we only capture a single layer, so hl with foreground is the preferred.

return aCol < bCol or (aCol == bCol and aPriority < bPriority)
end)

local virtText = {{'', 'UfoFoldedFg'}}
Copy link
Owner

Choose a reason for hiding this comment

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

If the first char is highlighted, the result will carry this non-sense chunk for the user.

-- get the most relevant mark
local mark = default
for _, m in ipairs(hlMarks) do
if (mark[6] <= m[6] and m[2] < i and i <= m[4]) then
Copy link
Owner

Choose a reason for hiding this comment

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

The priority of normal hl and conceal are at different levels. conceal should be higher than hl.

elseif not conceal and hlGroup ~= virtText[#virtText][2] then
table.insert(virtText, {char, hlGroup})
elseif not conceal then
virtText[#virtText][1] = virtText[#virtText][1] .. char
Copy link
Owner

Choose a reason for hiding this comment

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

The bad situation is the long folded line only with a single hl group, probably making a hundred concation.
That's why I use slots for the original implement to reduce concation. Just let you know my concern, you can ignore it for now.

@milisims
Copy link
Contributor Author

milisims commented Sep 5, 2023

Added synconcealed support & made the rest of the changes, happy to address anything else but this works very well in my testing

@kevinhwang91
Copy link
Owner

Should add concealLevel parameter to M.captureVirtText(bufnr, text, lnum, syntax, namespaces):
1: 0 will pass the conceal logic;
2: 1 and 2 show the cchar which is the first char of conceal extmark;
3: 3 hide the concealed range;

@milisims
Copy link
Contributor Author

I think this is good for merge unless there's anything else you can think of!

@kevinhwang91
Copy link
Owner

Thanks, I will append some commits in this PR. There are some details that are hard to say, you can check out the commit.

And I found an issue like below image:
image

After adding virtual text or conceal, the width of foldtext output is not fixed sometime.
The char text of foldtext output should map to the virtual text captured by ufo, otherwise, there is a flicker issue.
The reason is adding extmarks in a decorator render in the next tick, but foldtext renders in the current tick.

I will rework the module to fix this issue later.

@kevinhwang91
Copy link
Owner

Have appended some commits, feel free to review them. No issue will merge into main.

local res = {}
local prioritySlots = {}
local hlGroups = highlight.hlGroups()
local concealEabnled = concealLevel > 0
Copy link
Contributor Author

Choose a reason for hiding this comment

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

%s/concealEabnled/concealEnabled/g I assume? For this and the next function

@milisims
Copy link
Contributor Author

Other than typo looks great to me

@kevinhwang91 kevinhwang91 merged commit 16c730d into kevinhwang91:main Sep 20, 2023
@kevinhwang91
Copy link
Owner

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants