Repository navigation
Conversation
|
I am seeing errors in some runs in a CI suite across a wide range of targets, e.g.: https://ocaml.ci.dev/github/ahrefs/ocannl/commit/5c2fd68319a965307de8ed1eb719d98fde824116/variant/fedora-42-5.4_opam-2.4 The upside is that when this CI suite becomes green, I get more confidence in this PR. Edited to add: this is fixed now. |
|
I did not have the time to look at this yet. cc @MisterDA who may also want to have a look. |
| | C0 '\x1b' -> key `Escape [] | ||
| | C0 ('\b'|'\x7f') -> key `Backspace [] | ||
| | C0 '\n' -> key `Enter [] | ||
| | C0 ('\n'|'\r') -> key `Enter [] |
There was a problem hiding this comment.
We can probably affine the pattern matching with Sys.os_type.
There was a problem hiding this comment.
Isn't this reversed, too? The line ending is CRLF, \r\n.
There was a problem hiding this comment.
@MisterDA This is pattern alternative of characters, not string matching. Is there an actionable item here for me? It feels innocent to map '\r' to the Enter key but I could make it a when Sys.os_type = "Win32".
There was a problem hiding this comment.
This is pattern alternative of characters, not string matching.
Right, my mistake!
Is there an actionable item here for me?
Not sure. Have you observed the \r behavior on Windows?
In any case yes, you could restrict it to Windows and/or Cygwin.
There was a problem hiding this comment.
Why do we need to generate such keypresses for carriage returns in the first place? Shouldn't the new line be enough?
There was a problem hiding this comment.
In raw mode on Windows, the console delivers the actual carriage return character (\r) when Enter is pressed.
Here is how Claude defends my stance that the Sys.os_type condition would be less clean: "making it unconditional is simpler and accepting \r as Enter on all platforms seems harmless. Historically, \r (carriage return) was the "return" key anyway, so it's semantically correct.
Arguments for keeping it unconditional:
- Simpler code, no platform checks needed
\ris rarely sent by itself on Unix terminals in practice, so there's minimal risk of false positives- More robust - handles edge cases like users running Windows terminal emulators on Unix, WSL scenarios, or unusual terminal configurations"
There was a problem hiding this comment.
Ok but I prefer to have a comment at least, specially about a possible false positive for unix platform. I think that in the long run, we’ll probably come across this line of code again. Otherwise, I ok to merge it.
|
If this PR incorporates some of the code I submitted, I would appreciate being listed as a commit co-author. |
This commit adds native Windows support to the notty-community library: - **winsize.c**: Implement Windows console API support using GetConsoleScreenBufferInfo - Use srWindow (visible area) instead of buffer size for accurate terminal dimensions - Return 0 from winch_number on Windows (no SIGWINCH signal) - Add portable unused parameter macro for C23/GNU/other compilers - Include Haiku compatibility for platforms with ioctl in unistd.h - **notty_unix.ml**: Handle winch_number returning 0 on Windows - Prevent signal registration errors when SIGWINCH is unavailable - Return no-op revert function for platforms without window resize signals - **dune**: Add compiler-specific C flags - GCC/Clang: -Wall -Wextra -O3 - MSVC: /W2 /Ox - Use ocaml-config:ccomp_type to select appropriate flags Co-authored-by: Antonin Décimo <antonin@tarides.com> Co-authored-by: Ali Caglayan <alizter@gmail.com>
Signed-off-by: lukstafi <lukstafi@users.noreply.github.com>
On Windows, Unix.tcgetattr raises Invalid_argument instead of
Unix_error(ENOTTY) when terminal control functions are not implemented.
This caused crashes when using notty-community TUI tools on Windows.
Add exception handler for Invalid_argument to gracefully handle Windows
platforms where terminal attributes cannot be configured.
Fixes: Error: Invalid_argument("Unix.tcgetattr not implemented")
Co-authored-by: Antonin Décimo <antonin@tarides.com>
…ality This commit enables full terminal UI features on Windows 10 and later by activating the Virtual Terminal processing mode through the Windows Console API. **New file: win_console.c** - Implements caml_notty_setup_windows_console() to enable VT processing - Sets ENABLE_VIRTUAL_TERMINAL_PROCESSING for output (colors, cursor control) - Sets ENABLE_VIRTUAL_TERMINAL_INPUT for better keyboard input handling - Disables line input and echo for raw terminal input - Enables window input for resize event detection - Gracefully handles older Windows versions and non-console outputs **Updated: notty_unix.ml** - Calls setup_windows_console() when TERM variable is not set (typical on Windows) - Uses ANSI capabilities (Cap.ansi) when VT processing is successfully enabled - Falls back to dumb terminal mode if VT processing unavailable **Updated: dune** - Added win_console to the list of C stubs to compile **Benefits:** - ✅ Colors and text attributes now work on Windows - ✅ Cursor positioning and screen clearing work properly - ✅ Double-buffering via alternate screen buffer - ✅ Improved keyboard input handling - ✅ Window resize event detection Requires Windows 10 TH2 (v1511) or later. Older Windows versions will continue to work with degraded functionality (dumb terminal mode). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Windows terminals send carriage return '\r' (0x0D) for the Enter key, while Unix terminals send line feed '\n' (0x0A). Previously, '\r' was being misinterpreted as Ctrl+M due to the C0 control code fallback logic. Updated the Enter key pattern match to accept both '\n' and '\r', ensuring Enter key works correctly on both Windows and Unix platforms. Fixes: Enter key producing 'M' character on Windows 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Remove parentheses from the C23 [[maybe_unused]] macro expansion. The parentheses caused GCC to interpret the parameter as a type name, triggering -Wimplicit-int errors on Fedora 42/43 and openSUSE 16.0. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
Idea originators are now credited in the revised commits. |
|
Do mouse and keyboard events work? I believe @MisterDA's patch doesn't handle that but it was displaying. |
I have not attempted nor verified mouse events and keyboard events beyond regular typing, arrow keys, and [Enter]. Looking at the code, Claude says that events should work in general: "Based on the code, yes, both mouse and keyboard events should work on Windows 10+. The crucial piece is in win_console.c:48, where the code enables This is the key that makes Windows Console emit ANSI escape sequences for both keyboard and mouse input, rather than Windows-specific console events. Without this flag, the Windows Console won't send the CSI sequences that the mouse event parser in notty.ml:790-809 expects. The mouse handling enables several ANSI mouse modes on notty.ml:553-554: These enable various mouse tracking protocols (button press, drag, SGR mode, etc.). To summarize:
|
|
Friendly ping? Anyone feel free to create a new PR if there are any worries about this one. |
|
|
||
| #define __unit() value unit __attribute__((unused)) | ||
|
|
||
| CAMLprim value caml_notty_winch_number (__unit()) { |
There was a problem hiding this comment.
I'm not sure that this thunk should be a part of this PR, can you remove it (and probably propose such change into another PR)?
|
We probably should add a CI also for Windows with this PR. Let me know if you have time to add it, I have a little free time to participate, merge this PR and probably release the package on |
dinosaure
left a comment
There was a problem hiding this comment.
On cb1bed6, we probably should just copy/paste these lines which is used for several years now on many projects.
|
And a last thing, is it possible to keep ASCII charset on commit messages? |
This PR fixes #2 (winsize) drawing from the two commits mentioned there -- incorporates contributions from Antonin Décimo.
This PR also provides proper console support on the Windows OS -- courtesy of Claude. I verified the behavior via my project ppx_minidebug.
Vibe-coded using Claude Code.