Skip to content
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

Added Wrappers for Windows API functions GetConsoleMode and SetConsoleMode #19900

Closed
wants to merge 9 commits into from

Conversation

SeanTheGleaming
Copy link
Contributor

  • Added extern SetConsoleMode function to os.windows.kernel32
  • Added GetConsoleMode function to os.windows
  • Added SetConsoleMode function to os.windows
  • Added ConsoleMode type to std.os.windows
    • Returned by os.windows.GetConsoleMode
    • Passed into os.windows.SetConsoleMode
    • This is a packed union backed by a DWORD value
    • Has a field dword which is to be directly used in calls to the above API functions
    • Has a field input representing flags for console input buffers
    • Has a field screenbuf representing flags for console screen buffers
  • Removed ENABLE_VIRTUAL_TERMINAL_PROCESSING from os.windows
    • This is made redundant by ConsoleMode.screenbuf.ENABLE_VIRTUAL_TERMINAL_PROCESSING
  • Tweaked fs.supportsAnsiEscapeCodes to use this new interface

- Added `GetConsoleMode` function to `std.os.windows`
- Added `SetConsoleMode` function to `std.os.windows`
- Added `ConsoleMode` type to `std.os.windows`
  - It is passed into/returned by the above functions
  - This is a packed union backed by a `DWORD` value
  - Has a field `dword` which is to be directly used in calls to the above API functions
  - Has a field `input` representing flags for console input buffers
  - Has a field `screenbuf` representing flags for console screen buffers
- Removed `ENABLE_VIRTUAL_TERMINAL_PROCESSING` from `std.os.windows`
  - This is made redundant by `ConsoleMode.screenbuf.ENABLE_VIRTUAL_TERMINAL_PROCESSING`
Modified to utilize new interface for `GetConsoleMode` on Windows
@SeanTheGleaming SeanTheGleaming marked this pull request as draft May 8, 2024 19:04
Made all fields of `ConsoleMode` the same bit-width (`@bitSizeOf(DWORD)`)
@squeek502
Copy link
Collaborator

See #18715 for a related PR and some history of the SetConsoleMode binding.

@SeanTheGleaming
Copy link
Contributor Author

SeanTheGleaming commented May 10, 2024

As it stands, the two main things holding me back from submitting this PR as is are:

  1. Most obviously, the checks for windows don't pass, and the logs for the action provide very little info. Considering that none of these changes should be breaking for the wider std, and that at one point a single Mac check failed but but then resolved itself for no discernible reason, especially given this is Windows specific code, makes me tempted to think that these checks may have failed in some sort of error, but I don't want to be so quick to jump to that conclusion, so I want to find out what might be causing this. However, the previously mentioned lack of detail in the action logs is making this difficult, as it compiles fine on my machine
  2. I simply haven't been able to find any documentation ENABLE_AUTO_POSITION. The closest I've found is a few articles that mention it in passing, only remarking that it in undocumented. If anyone has details about the ENABLE_AUTO_POSITION flag, any information would be appreciated, as I am adding doc comments to each field of ConsoleMode

@alexrp
Copy link
Contributor

alexrp commented May 10, 2024

  1. I simply haven't been able to find any documentation ENABLE_AUTO_POSITION. The closest I've found is a few articles that mention it in passing, only remarking that it in undocumented. If anyone has details about the ENABLE_AUTO_POSITION flag, any information would be appreciated, as I am adding doc comments to each field of ConsoleMode

microsoft/terminal#15685 (reply in thread)

@squeek502
Copy link
Collaborator

  • I think a change you've made is inadvertently exposing/triggering a bug in the C backend (zig1.exe trying to compile Zig to zig2.c using the C backend is what's failing). I can reproduce the CI failures locally when trying to build Zig from source in this branch.
  • lib/std/os/windows/kernel32: add signature for SetConsoleMode #18715 has been merged, so you'll need to rebase and fix the conflicts

Reading through the uses of it in https://github.com/microsoft/terminal, it seems that the main effect of this flag is to pass `CW_USEDEFAULT` into the `CreateWindowExW` parameter `X`, so documentation has been added regarding the effects of this according to https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-createwindowexw
Tweaked the doc comment on `ConsoleMode` to make it more clear that the `dword` field is for usage with the raw, unwrapped `kernel32.SetConsoleMode`/`kernel32.SetConsoleMode` functions and not their Zig-style bindings in `std.os.windows`
@SeanTheGleaming
Copy link
Contributor Author

SeanTheGleaming commented May 13, 2024

#18715 has been merged, so you'll need to rebase and fix the conflicts

While resolving the conflicts, I removed the DISABLE_NEWLINE_AUTO_RETURN declaration, as in this PR, it would be made redundant by the ConsoleMode.screenbuf.DISABLE_NEWLINE_AUTO_RETURN field, although I would like to hear if @Garfield550 wants to weigh in on this

@SeanTheGleaming
Copy link
Contributor Author

Closing this PR for the time being, as progress has been slow, and it is currently based off of my master branch (I thought this would be much simpler than it turned out to be)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants