search

Yomguithereal / react-blessed

A react renderer for blessed.
MIT License
4.46k stars 177 forks source link

Add createBlessedRenderer function #80

Closed d4rky-pl closed 6 years ago

d4rky-pl commented 6 years ago

Fixes #79

The diff looks long mostly because of whitespace changes, there are no major modifications to the code.

Yomguithereal commented 6 years ago

Thanks @d4rky-pl. I will need some time to read this. Just discovered neo-blessed. Didn't know about this. Don't hesitate to ping me if I forget.

d4rky-pl commented 6 years ago

@Yomguithereal someone linked it either in issues here or in original blessed. The diff looks complicated but it's basically moving everything below type Instance into a function so blessed is passed from outside and not from require and then returning what used to be the old render function.

If it helps I can split it into two commits, one without adding indentation (which is what destroyed github diff view) and then second that fixes indentation. Just let me know!

d4rky-pl commented 6 years ago

It just occured to me that the way I wrote this would make blessed still a hard dependency on the package so I pushed a fix.

Which reminds me: how much time are you willing to spend on this library and how much it was just an experiment that suddenly got popular? I have more ideas on how to improve the codebase, builds, bundle size, APIs etc but I don't know if it makes sense to start discussions, send PRs etc. If you're busy and/or bored with this I can always either A) fork or B) create my own library and hope it gets as many github stars 😜

Yomguithereal commented 6 years ago

Everything looks good to me. Can you add some docs about the newly created function though, please?

I have more ideas on how to improve the codebase, builds, bundle size, APIs etc but I don't know if it makes sense to start discussions, send PRs etc.

Don't hesitate to do so. I don't have much time lately but contributions are always welcome.

d4rky-pl commented 6 years ago

I've rebased the branch to fix the merge conflict and updated README

Yomguithereal commented 6 years ago

Thanks @d4rky-pl. I'll merge now.