-
Notifications
You must be signed in to change notification settings - Fork 140
Adds native Windows support #200
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
base: master
Are you sure you want to change the base?
Conversation
choices.c: Win32 processor count
tty.h: Semantic change for tty_newline: No longer clears, and serves as platform LF/CRLF
tty_interface.c: Prior hardcoded '\n' are now calls to tty_newline. Additionally, prior calls to tty_newline is now preceded by tty_clearline
tty.c -> tty_posix.c: The only change to tty.c (other than renaming) is that tty_newline no longer clears the line
tty_win32.c: Simple windows tty support. Considerations:
- Uses ASCII versions of CreateFile, ReadConsole, and WriteConsole. For future reference consult {Create,Read,Write}{Console,File}{A/W/Input}
- tty_init doesn't implement tty_filename argument on Windows, only CONIN$/CONOUT$
- Buffered IO: Buffer size (setvbuf) not set. FILE* vs. HANDLE (for conmode)
- SIGWINCH not handled. For future reference, see ReadConsoleInput + WINDOW_BUFFER_SIZE_EVENT
Makefile: OS check
|
I forgot to comment on 949d08e ("Use opaque pointer for tty_t in preparation for Windows support"). Most importantly this builds on EDIT Below are same comments as above but attached to respective files. |
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 went through several alternatives before settling on renaming tty.c and the conditional in the Makefile. For compatibility with other forks it may be desirable to keep tty.c and change the non-Windows branch in the Makefile to TTY=tty. Or to keep a tty.c with the following contents:
#ifdef __linux__
#include "tty_posix.c"
#elif _WIN32
#include "tty_win32.c"
#endif
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.
Semantic change for tty_newline: No longer clears, and serves as platform LF/CRLF.
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.
Let tty_init pass an opaque pointer to caller. Move struct termios and termios.h —which are not on Windows—from tty.h to tty.c (renamed tty_posix.c).
See struct conmode and struct tty in tty_win32.c for how I utilized the encapsulation.
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.
Prior hardcoded '\n' are now calls to tty_newline. Additionally, prior calls to tty_newline are now preceded by tty_clearline since tty_newline no longer clears.
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.
Semantic change for tty_newline: No longer clears, and serves as platform LF/CRLF.
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.
Simple Windows tty support, sticking as close as possible to present implementation in tty.c (renamed to tty_posix.c).
Considerations:
- Uses ASCII versions of
CreateFile,ReadConsole, andWriteConsole. For future reference consult{Create,Read,Write}{Console,File}{A/W/Input} tty_initdoesn't implementtty_filenameargument, onlyCONIN$/CONOUT$- Buffered IO: Buffer size (
setvbuf) not set.FILE*vs.HANDLE(for conmode) SIGWINCHnot handled. For future reference, seeReadConsoleInput+WINDOW_BUFFER_SIZE_EVENT- Anticipates Tighten the
ttyplatform abstraction #199 and implementstty_puts.
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.
tty_init now returns an opaque pointer to caller.
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.
Let tty_init pass an opaque pointer to caller. Move struct termios and termios.h —which are not on Windows—from tty.h to tty.c (renamed tty_posix.c).
See struct conmode and struct tty in tty_win32.c for how I utilized the encapsulation.
This is a concise patch bringing simple native Windows support to
fzy.I have tried to keep the changes to absolute minimum and refrained from including fixups in this PR.
It may be interesting to diff
tty_posix.cwithtty_win32.cfor a quick overview of the port.What I'm most unsure about is if you'll find the conditional build OK. I went through several alternatives before settling on renaming
tty.cand the conditional in theMakefile. For compatibility with other forks it may be desirable to keeptty.cand change the non-Windows branch in theMakefiletoTTY=tty. Or to keep atty.cwith the following contents:Let me know if you're otherwise fine with the changes but would like them integrated in the build differently. I can prepare it how you are most comfortable.
Needs testing on POSIX-ish. Tested on Windows 11 and functional. Wants further enhancements to mitigate flickering (will do that in a separate PR: #201 ).
747e26d anticipates #199 and implements
tty_puts.Commit message: