Conversation
windows support has been requested several times throughout the years, however i've always been reluctant to pollute the library with ifdefs and/or abstraction layers for this sake. we avoid this with a shell script (`windows.sh`) that does surgery on `termbox2.h` to generate a windows-compatible version. some headers are swapped, some functions are altered or replaced, but most of the library stays the same. a couple marker comments (`__posix_(start|end)`) are added to `termbox2.h` which `windows.sh` uses to anchor various sed/awk commands. theoretically the test suite could run on windows. it would take some research to figure out the windows equivalents of things in `tests/run.sh` like `xvkbd` etc. github docs state that `windows-latest` has php pre-installed, and php does have windows ffi support. the memfd-based tests would have to be skipped (or proper rwfd support would need to be added to the windows lib). instead of running the test suite, we ensure we can at least compile the keyboard demo on windows. in my opinion this suffices for experimental support.
|
Looking with an eye towards implementing wasm support which is very similar to posix, but will have different "cut points" than what is required for windows, so we would need to have separate places for _start/_end, and then we're back to ifdef by another name. It makes me wonder if it wouldn't be possible to define a host interface which was the minimal set of functions required for termbox2 to interact with its environment. For example, I imagine that signal handling will be different on posix, windows, and wasm, so rather than swapping out the implementation with sed, you can have a host specific function that is delegated to. E.g. tb_host_init();
tb_host_denit();
tb_host_on_resize(handle_resize);What it means to init, de-init, and install a signal handler is different on all three hosts (I've only listed those three, but there would be more). With respect to signal handling for example: Posix listens for SIGWINCH, windows does lord knows what, and with WASM it has to be provided via an externally linked function by Node or the browser. Then, you could have a single section that you could swap out, right after the includes, that would have everything related to the side-effects of the host environment. An added advantage of this approach would be that the "patch" could be stored as a normal header file which would make long term maintenance much less painful. Just a thought. It probably does entail some refactoring, but I think that having side-effects behind a well defined API will make the code much more readable while at the same time supporting subtle differences in host environment. |
|
Just a test report, used this successfully to port my Nim TUI app to Windows (using the new Windows Terminal / Powershell). Thanks! |
Excellent. Thank you for testing. |
|
You did this to minimize ifdefs, but aren't posix start/end comments ifdefs in disguise? Also imho this makes the single implementation header file less single implementation. You need to ship two distinct windows and a linux header files for this to work as well as a msys2 environment if you wanna compile this on windows (because AFAIK Windows doesn't have sed/awk natively). |
Sort of but they avoid the big else blocks for Windows code which IMO harms the readability of the main use-case of termbox, which is POSIX. The previous 2 PRs for Windows support were ifdef-based. I never felt good about merging them for this reason. The problem compounds if we include WASM support.
Good points. I don't love this solution either, but I still prefer it over ifdefs or going multi-file. I place high value on readability. A few small ifdefs here or there are fine IMO, but when replacing entire functions it starts getting ugly and harder to follow. If it weren't a single-file library, there are ways we could make it cleaner. However I also don't want to sacrifice single-file for the sake of readability. That's how I arrived at the solution in this PR. My view might change if lots of Windows developers started using termbox. In absence of that it's cleaner to optimize the experience for POSIX. It's a good point that Windows users won't have sed or awk. There are options for them though -- msys2 as you mentioned, WSL, Cygwin. I don't interact with Windows developers enough to know how much friction that introduces. (On my Windows machine, the first thing I did was install WSL.) Would it prevent you from using the library? EDIT: We could rewrite the code gen script in a language with Windows support like PHP. My guess is that'd introduce more friction than requiring WSL etc. We could include a pre-generated Windows header in the repo, though I'm less inclined. Making it a downloadable artifact in GitHub CI is another option. Thank you for the feedback. |
My idea might be naive, but doesn't we only need 2
You basically already are multifile, because now code concerning windows is bundled in
This sounds a bit like the chicken or the egg problem. Not a lot Windows devs will start using termbox if it is not cross platform yet.
I've created Lua bindings to the library using the previous PR, which still lies unpublished in some private repo. LuaRocks is the defacto package manager for Lua is available on msys2 and native windows (as well as all unix systems). This means executing a script beforehand that uses unix like tools is only possible if the Windows installation of LuaRocks comes from msys2. It would not prevent using the library, but if you need to install additional tooling, the pool of potential users gets smaller.
When it comes to packaging then at least for windows pre-generated header files make it much easier (if we need unix tools). I personally would use the two
No problem. Take my comments as mere suggestion, as I am currently not really using termbox. Take what you think is valuable and toss the rest! I wanted to create a CLI shell for Lua that does autocompletion popups while still running within the shell (not resetting the whole screen) and the libraries I found were unix only or had a dependency hell; and since TB2 does not support non fullscreen apps, I've opted to implement the TTY communication myself. I have a few fullscreen CLI ideas, for which I will probably go back to TB2, because ncurses is a dependency hell as well. And then maybe I'll finally publish the bindings. |
|
I can maybe warm up to that idea. Thank you again for the input. I'll leave this PR open for more feedback. And maybe open a PR that implements the above.
If you haven't seen it already, check out #114. I didn't find an obvious portable way to scroll the primary screen. The PR seems to work in xterm, vte, and the Linux console. I'd like to test it on other terminals before merging. The other open question there is whether to support x-offset and variable width inline TUIs. I'm not sure if there's any practical use for that to be worth the complexity. As written it only support full-width (and variable height of course). |
I would personally advise against using platform specific header files. If this library is really meant to be cross platform then the platform specific details should not be visible in the API (and
Ohh that's exactly what I was looking for thanks for pointing that out to me! I haven't seen it, partly because my project is older than the PR =) |
Could we have our cake and eat it too by forward declaring the platform api, and then just dropping in both platform specific |
Coming back to this, I realize that is exactly what @sewbacca was suggesting. Sorry, I didn't get that.
I'm cool with either way, as long as it would be possible to have a What would we need to do to move this forward? I'd love to make this happen. |
|
Assuming we're going multi-file, and assuming we want to keep offering amalgamated single-file versions, the remaining question is whether we include amalgamated single-file versions in source control or not. Most projects copy termbox2.h into their repo. Those projects would either need to start copying multiple files, or run Other projects include termbox2 as a git submodule. If we stop including single-file versions in the repo, those builds would break the next time they update. Maybe we could offer back-compat with something like: // termbox2.h
#include <termbox2_base.h>
#ifdef TB_IMPL
#include <termbox2_posix.c>
#endif`Either way, multi-file would be a big change. I'm not totally against it. I'd personally like to hear input from more library users. If we add all this complexity, and only one or two small projects end up using the library outside POSIX, maybe it wouldn't be worth it. If it unlocks some huge potential somewhere, then yeah maybe worth it. |
|
Hi, I thought I'd just throw some input as a user of termbox2 :) The current approach with the generator script doesn't look too friendly to work with. Imagine trying to fiddle with the logic on the for-Windows implementation part. I understand those are Windows users we are talking about but they're still people you know what I mean :D In my opinion the best thing would be to keep code in the files for code, yeah. Also I think that it'd be nice to keep living up to the standard of single-file header-only library, it's very convenient to use it that way. Yeah there'll be ifdefs but if done properly this is not that big of a deal (raw ifdefs sprinkled all over the file are obviously a bad thing - to avoid that, platform specific logic must be wrapped in some Also I imagine that a use case of providing both Linux and Windows support with termbox2 in some terminal app project is much better off if it's all bundled in one file. Having to manage multiple termbox2 files in your build system recipe is a great pain in the... who likes touching their build recipes, anyways ;) Best regards, Grigory |
|
What would be the disadvantage of making one |
I am unsure what goes against a A naive solution that scales with #ifdef TB_IMP
// setup platform detection
#ifdef TB_POSIX
// include posix headers
#elif TB_WIN
// include windows headers
#endif
#endif // TB_IMP headerblock
// <public includes>
// <public API definitions>
#ifdef TB_IMP
// abstract internal API definitions for OS specific calls
// implementation for public API
#ifdef TB_POSIX
// platform specific implementation for internal API on POSIX
#elif TB_WIN
// platform specific implementation for internal API on Windows
#endif // TB_<plat> block
#endif // TB_IMP implementation blockFor each additional platform only 2 You could alternatively reduce it to // <public includes>
// <public API definitions>
#ifdef TB_IMP
// abstract internal API definitions for OS specific calls
// implementation for public API
// detect platform
#if defined(TB_POSIX)
// posix includes
// platform specific implementation for internal API on POSIX
#elif defined(TB_WIN)
// windows includes
// platform specific implementation for internal API on Windows
#endif // TB_<plat> block
#endif // TB_IMP implementation blockThis may or may not work for Windows as there are some constraints at which point you can include Alternatively this skeleton scales #ifdef TB_IMP
// detect platform
#ifdef TB_WIN
#include<windows.h>
#endif
#endif // detection and windows.h workaround
// <public includes>
// <public API definitions>
#ifdef TB_IMP
// abstract internal API definitions for OS specific calls
// implementation for public API
#if defined(TB_POSIX)
// posix includes
// platform specific implementation for internal API on POSIX
#elif defined(TB_WIN)
// windows includes
// platform specific implementation for internal API on Windows
#endif // TB_<plat> block
#endif // TB_IMP implementation block |
|
I opened a proof-of-concept that demonstrates what splitting out platform specific files could look like here #129 It's comprised of a It's almost certain that I've picked the wrong abstractions since I don't really understand the intricacies of how the library fits together and so am not quite sure where to draw the lines, but the point is to mainly demonstrate the developer experience which unlocks mulitple platforms, while preserving a) single |
5 participants
windows support has been requested several times throughout the years, however i've always been reluctant to pollute the library with ifdefs and/or abstraction layers for this sake. we avoid this with a shell script (
windows.sh) that does surgery ontermbox2.hto generate a windows-compatible version. some headers are swapped, some functions are altered or replaced, but most of the library stays the same. a couple marker comments (__posix_(start|end)) are added totermbox2.hwhichwindows.shuses to anchor various sed/awk commands.theoretically the test suite could run on windows. it would take some research to figure out the windows equivalents of things in
tests/run.shlikexvkbdetc. github docs state thatwindows-latesthas php pre-installed, and php does have windows ffi support. the memfd-based tests would have to be skipped (or proper rwfd support would need to be added to the windows lib).instead of running the test suite, we ensure we can at least compile the keyboard demo on windows. in my opinion this suffices for experimental support.