nui.nvim icon indicating copy to clipboard operation
nui.nvim copied to clipboard

feat: implement border text function

Open romgrk opened this issue 3 years ago • 4 comments

Link: https://github.com/MunifTanjim/nui.nvim/pull/142

romgrk avatar Jul 13 '22 11:07 romgrk

Makes sense that you prefer to have a function here. I'd like that function to return string | string[] | NuiText | NuiText[] instead of NuiLine (which represents a whole Line in a buffer). And mid_char should be passed as 2nd param to that function.

I've added the mid_char parameter.

For the polymorphic return type, is it really necessary? For the simple use-cases, the user already has access to border:set_text(edge, text). Making the rendering function have multiple return types will complicate the code without offering a clear advantage, imho.

romgrk avatar Jul 13 '22 11:07 romgrk

Codecov Report

Merging #146 (b7d7bfc) into main (5a79b1b) will increase coverage by 0.01%. The diff coverage is 93.75%.

@@            Coverage Diff             @@
##             main     #146      +/-   ##
==========================================
+ Coverage   97.12%   97.14%   +0.01%     
==========================================
  Files          15       15              
  Lines        1670     1681      +11     
==========================================
+ Hits         1622     1633      +11     
  Misses         48       48              
Impacted Files Coverage Δ
lua/nui/popup/border.lua 95.72% <85.71%> (+0.03%) :arrow_up:
lua/nui/line/init.lua 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 5a79b1b...b7d7bfc. Read the comment docs.

codecov[bot] avatar Jul 13 '22 13:07 codecov[bot]

For the polymorphic return type, is it really necessary?

Making the rendering function have multiple return types will complicate the code without offering a clear advantage, imho.

Hmm... Yep, you are right. Let's leave that polymorphic return from here.

MunifTanjim avatar Jul 13 '22 13:07 MunifTanjim

@romgrk I've added some minor feedbacks. Other than that it looks good 👍🏼 We can merge after those are addressed.

And thanks for coming up with this use-case. 😃

MunifTanjim avatar Jul 13 '22 13:07 MunifTanjim

Closing, stale.

romgrk avatar Apr 19 '24 10:04 romgrk