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(foldtext): add option to get virt texts of arbitrary rows #74

Merged
merged 14 commits into from
Sep 10, 2022

Conversation

aileot
Copy link
Contributor

@aileot aileot commented Sep 6, 2022

The option converts the first argument of virt text handler into a list of virt texts on Lua metatable.
It's useful when the first lines of folds give little information.

For example, we can see the second lines that show plugin names managed by plugin manager at a glance:

  1. packer.nvim
use {  'kevinhwang91/nvim-ufo',  2
}

for

use {
    'kevinhwang91/nvim-ufo',
    requires = 'kevinhwang91/promise-async'
}
  1. dein.vim in toml
[[plugins]]  repo = 'kevinhwang91/nvim-ufo'  2

for

[[plugins]]
repo = 'kevinhwang91/nvim-ufo'
depends = 'kevinhwang91/promise-async'

A sample handler in which folded lines will be concatenated until each virtText becomes longer than minWidth:

--- A list of [`text`, `highlight`] tuples.
---@alias VirtText string[][]

---@param virtText VirtText
---@return number
local virtTextWidth = function(virtText)
  local width = 0
  for _, c in ipairs(virtText) do
    local chunkText = c[1]
    width = width + vim.fn.strdisplaywidth(chunkText)
  end
  return width
end

---@param virtText VirtText
---@return VirtText
local trimIndent = function(virtText)
  local newVirtText = vim.deepcopy(virtText)
  local firstChunkText = newVirtText[1][1]
  newVirtText[1][1] = firstChunkText:gsub("^%s+", "")
  return newVirtText
end

---@param virtText VirtText
---@return VirtText
local compressSpaces = function(virtText)
  local newVirtText = vim.deepcopy(virtText)
  for _, c in ipairs(newVirtText) do
    local chunkText = c[1]
    c[1] = chunkText:gsub("%s+", " ")
  end
  return newVirtText
end

--- Combine two virtual texts.
---@param a VirtText
---@param b VirtText
---@return VirtText
local combineVirtTexts = function(a, b)
  local newVirtText = vim.deepcopy(a)
  for _, chunk in ipairs(b) do
    table.insert(newVirtText, chunk)
  end
  return newVirtText
end

--- Concatenate multiple virtual texts.
---@param a VirtText
---@vararg VirtText
---@return VirtText
local concatVirtTexts = function(a, ...)
  local newVirtText = vim.deepcopy(a)
  for _, b in ipairs({ ... }) do
    newVirtText = combineVirtTexts(newVirtText, b)
  end
  return newVirtText
end

local handler = function(virtTexts, lnum, endLnum, width, truncate)
  local virtText = virtTexts[lnum]
  local minWidth = 25
  local offset = 0
  local separator = { { "", "Delimiter" } }
  local maxOffset = endLnum - lnum
  while virtTextWidth(virtText) < minWidth and offset <= maxOffset do
    offset = offset + 1
    virtText = concatVirtTexts(
      virtText,
      separator,
      compressSpaces(trimIndent(virtTexts[lnum + offset]))
  end
  local newVirtText = {}
  local suffix = ('  %d '):format(endLnum - lnum)
  -- The rest would be the same as the example in README.md.
end

require('ufo').setup({
    enable_on_demand_virt_texts = true -- The new option. (default: false)
    -- enable_fold_end_virt_text = true, -- It could be deprecated.
    fold_virt_text_handler = handler
})

In addition, I also suggest such functions to deal with virtTexts as above be provided for users by, for instance, require('ufo.user-utils').

@kevinhwang91
Copy link
Owner

kevinhwang91 commented Sep 6, 2022

Not bad, but should pass the metatable into 6th parm for the handler instead of making 1st parm confusing for users.

For instance, if 3rd plugin author needs to integrate ufo with his buffer, he will call setFoldVirtTextHandler(bufnr, handler), but the 1st type of parm depends on ufo's config, it's so confusing.

BTW, I don't know why the name is enable_on_demand_virt_texts, could you explain patiently?

In addition, I also suggest such functions to deal with virtTexts as above be provided for users by

Not yet, I don't think it's useful for most users.

@kevinhwang91 kevinhwang91 self-requested a review September 6, 2022 12:49
lua/ufo/decorator.lua Outdated Show resolved Hide resolved
lua/ufo/decorator.lua Outdated Show resolved Hide resolved
lua/ufo/decorator.lua Outdated Show resolved Hide resolved
@aileot
Copy link
Contributor Author

aileot commented Sep 7, 2022

BTW, I don't know why the name is enable_on_demand_virt_texts, could you explain patiently?

enable_virt_text_list the first alternative I think of sounds as if extra calculations have run for every lines in fold to make virt_text lists before handler while "on-demand" explicitly demands no unnecessary calculations. The doubt of extra resource consumption with the former name makes me feel uncomfortable beyond reason. Did this answer your question?

Anyway, you can rename it better. You're the author.

@kevinhwang91
Copy link
Owner

After considering return a list can't use ipairs and paris to iter (__ipairs and __pairs doesn't exist in Lua 5.1), I think exporting a function will be better. Free feel to share your opinion here before merging.

@aileot
Copy link
Contributor Author

aileot commented Sep 8, 2022

I agree with you. A function is more concise than a list in this case.

I'd like to cut off "func" from the function name. It's redundant. Still, it's hard to rename it better.

  • get_folded_line is to return a string.
  • get_folded_text is to return a string.
  • get_folded_node reminds me of tree-sitter node, which is distinct from what we deal with.
  • get_folded_chunks is used in help file, but it's plural.
  • get_folded_context is ambiguous.
  • get_folded_content implies all the lines in a fold, not a line.

Provisionally, I rename it get_fold_virt_text, which comes from the option name. Either this or get_folded_chunks seems better in them.

What do you think of it?

@kevinhwang91
Copy link
Owner

I agree with you. A function is more concise than a list in this case.

I'd like to cut off "func" from the function name. It's redundant. Still, it's hard to rename it better.

  • get_folded_line is to return a string.
  • get_folded_text is to return a string.
  • get_folded_node reminds me of tree-sitter node, which is distinct from what we deal with.
  • get_folded_chunks is used in help file, but it's plural.
  • get_folded_context is ambiguous.
  • get_folded_content implies all the lines in a fold, not a line.

Provisionally, I rename it get_fold_virt_text, which comes from the option name. Either this or get_folded_chunks seems better in them.

What do you think of it?

Look good, please change "" to '' and improve the docs, thanks a lot.

@aileot
Copy link
Contributor Author

aileot commented Sep 9, 2022

Sorry for the violation.

@aileot
Copy link
Contributor Author

aileot commented Sep 9, 2022

Force-pushed because the last two commits in doc/example.lua had got conflicted to that of origin/main. Please make sure your last changes are not overwritten there.

Well, may I add the screenshot in README.md?

ufo-dein-toml-sample
i

@kevinhwang91
Copy link
Owner

Don't change the struct of doc/exmaple, do-end will increase unnecessary indentations. enable_get_fold_virt_text_func should be changed to enable_get_fold_virt_text.

Well, may I add the screenshot in README.md?

Sorry, please don't. I want to keep the README.md as clean as possible for users.

@aileot
Copy link
Contributor Author

aileot commented Sep 10, 2022

No problem?

Copy link
Owner

@kevinhwang91 kevinhwang91 left a comment

Choose a reason for hiding this comment

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

So many detail for example

doc/example.lua Outdated Show resolved Hide resolved
@aileot
Copy link
Contributor Author

aileot commented Sep 10, 2022

Is that all?

@kevinhwang91 kevinhwang91 merged commit 7cf9d2a into kevinhwang91:main Sep 10, 2022
@kevinhwang91
Copy link
Owner

Thanks a lot, I have squashed and merged it.

@aileot aileot deleted the feat/on-demand-virt-texts branch September 11, 2022 12:51
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