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: add support for rendering text with props #465

Merged

Conversation

brandon1024
Copy link
Contributor

@brandon1024 brandon1024 commented Feb 14, 2023

Introduce the ability to render text with attached text properties. Renderers may now return list of dictionaries where each line of text can include text properties for additional text styling capabilities. This change is backwards compatible with existing renderers; they will continue to work as before.

The semantics of this feature was inspired by popup_create(), which can accept a list of dictionaries with a 'text' and 'props' key.

This feature enhances the capabilities of custom renderers. In particular, it's possible to implement renderers that render content with dynamically-defined style. The screenshot below shows a simple custom renderer that renders vim files in green, config files in grey and others in orange.

image

Here's a renderer I built using these features: brandon1024/fern-renderer-nf.vim

A very dumb renderer example:

" Build and return the renderer.
function! fern#renderer#nf#new() abort
        return {
		\ 'render': funcref('s:render'),
		\ 'index': { lnum -> lnum - 1 },
		\ 'lnum': { index -> index + 1 },
		\ 'syntax': { -> v:null },
		\ 'highlight': { -> v:null }
	\ }
endfunction

" Render given nodes and return a list of lines with text properties.
function! s:render(nodes) abort
	let l:base = len(a:nodes[0].__key)

	if empty(prop_type_get('test'))
		call prop_type_add('test', { 'highlight': 'Normal' })
	endif

	return map(copy(a:nodes), { i, node ->
		\ {
			\ 'text': '>' . repeat(' ', len(node.__key) - l:base) . node.label,
			\ 'type': [{ 'col': 1, 'length': 1, 'type': 'test' }]
		\ }})
endfunction

@lambdalisue lambdalisue self-requested a review February 20, 2023 07:09
@lambdalisue lambdalisue added the enhancement New feature or request label Feb 20, 2023
Copy link
Owner

@lambdalisue lambdalisue left a comment

Choose a reason for hiding this comment

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

Not sure but does this work on Neovim as well?

@brandon1024
Copy link
Contributor Author

@lambdalisue I don't believe so. From what I've read, extmarks is the neovim-equivalent to textprops but they are not compatible. This issue describes why textprops weren't introduced in neovim.

I guess the next question is where we go from here 😅 We might be able to build a shim that uses textprops for vim and extmarks for neovim, so that these features can be used for both editors. Alternatively we could document that textprops are only supported in vim. What are your thoughts? I'd love to support both but I have literally no experience with Neovim, so I'd need some guidance. Documenting that textprops are only supported for Vim seems the easiest to me.

@lambdalisue
Copy link
Owner

Alternatively we could document that textprops are only supported in vim

It sounds reasonable enough. It's OK if this change does not break current features in Neovim (I guess the current code breaks features on Neovim, right?)

@brandon1024
Copy link
Contributor Author

brandon1024 commented Feb 21, 2023

It sounds reasonable enough.

Cool, this makes my life easier. I'm sure someone will be interested in implementing this for extmarks at some point too.

It's OK if this change does not break current features in Neovim (I guess the current code breaks features on Neovim, right?)

Existing renderers should continue to work as before, for both vim and neovim 😃 The changes I've introduced check to see if it was passed a list of strings or a list of dictionaries. If passed dictionaries, it would fail, but that would be a bug on the renderer's part. I can also add a check to see if we are in vim or neovim and throw a useful error message too, or log a message. I might do that actually, not a bad idea.

@brandon1024
Copy link
Contributor Author

I've adjusted the docs, indicating that text properties are only supported in vim. I also added a check and a useful error message when trying to render text properties in neovim.

I guess the next question for you @lambdalisue , would you prefer that we fail abruptly if trying to use text properties in neovim, or just skip the text properties and render a plain-text tree? In code:

    if !empty(props) && !exists('*prop_add')
      throw 'text properties are not supported, prop_add() does not exist.'
    endif

    for prop in props
      let prop.bufnr = a:bufnr
      call prop_add(lnum + 1, prop.col, prop)
    endfor

or

    if exists('*prop_add')
      for prop in props
        let prop.bufnr = a:bufnr
        call prop_add(lnum + 1, prop.col, prop)
      endfor
    endif

I've thought about it a bit, I think the second one makes more sense, since we'd still render a file tree but it just wouldn't be colorized.

@lambdalisue
Copy link
Owner

or just skip the text properties and render a plain-text tree

Seems nice.

Additionally, I just moved so I need to find time to check PRs so please be patient. I'm sorry for that.

@brandon1024
Copy link
Contributor Author

No worries :-)

@brandon1024
Copy link
Contributor Author

@lambdalisue I think this is ready to go now, whenever you get a second :-)

Comment on lines +16 to +37
for lnum in range(len(a:content))
let line = a:content[lnum]
let [text, props] = type(line) is# v:t_dict
\ ? [line.text, get(line, 'props', [])]
\ : [line, []]

call setbufline(a:bufnr, lnum + 1, text)

if exists('*prop_add')
for prop in props
let prop.bufnr = a:bufnr
call prop_add(lnum + 1, prop.col, prop)
endfor
endif
endfor
Copy link
Owner

Choose a reason for hiding this comment

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

The use of for loops in Vim scripts has a significant impact on performance, so this part of the script should be optionally controllable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've wrapped this new functionality in a feature flag, which is disabled by default :-)

Introduce the ability to render text with attached text properties.
Renderers may now also return list of dictionaries where each line of
text can include text properties for additional text styling
capabilities. This change is backwards compatible with existing
renderers; they will continue to work as before. These features,
however, are not supported in Neovim.

The semantics of this feature was inspired by popup_create(), which can
accept a list of dictionaries with a 'text' and 'props' key.
@@ -74,7 +74,7 @@ function! s:show() abort
call setbufline('%', 1, line)
call helper.fern.renderer.syntax()
call helper.fern.renderer.highlight()
syntax clear FernRoot
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm pretty sure this is a bug. It was giving me some trouble so I fixed it here :-)

Copy link
Owner

@lambdalisue lambdalisue left a comment

Choose a reason for hiding this comment

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

LGTM

@lambdalisue lambdalisue merged commit dae5eb2 into lambdalisue:main Mar 10, 2023
@lambdalisue
Copy link
Owner

Thanks for your contribution 👍

@brandon1024
Copy link
Contributor Author

Thanks @lambdalisue :-)

@brandon1024 brandon1024 deleted the renderer-support-textprops branch March 10, 2023 19:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants