-
-
Notifications
You must be signed in to change notification settings - Fork 48
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
feat: add support for rendering text with props #465
Conversation
There was a problem hiding this 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?
@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. |
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?) |
Cool, this makes my life easier. I'm sure someone will be interested in implementing this for extmarks at some point too.
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. |
2e37eb7
to
79d6b2d
Compare
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. |
Seems nice. Additionally, I just moved so I need to find time to check PRs so please be patient. I'm sorry for that. |
No worries :-) |
79d6b2d
to
10ddd45
Compare
@lambdalisue I think this is ready to go now, whenever you get a second :-) |
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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
10ddd45
to
21c0472
Compare
@@ -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 |
There was a problem hiding this comment.
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 :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Thanks for your contribution 👍 |
Thanks @lambdalisue :-) |
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.
Here's a renderer I built using these features: brandon1024/fern-renderer-nf.vim
A very dumb renderer example: