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

delete std.os.windows.user32 #17763

Merged
merged 1 commit into from
Oct 28, 2023
Merged

delete std.os.windows.user32 #17763

merged 1 commit into from
Oct 28, 2023

Conversation

andrewrk
Copy link
Member

Please use a third party package such as the excellent zigwin32gen project if you want to use the Windows API directly.

This is progress towards #4426
Closes #17417

This is progress towards #4426
Closes #17417
@andrewrk andrewrk enabled auto-merge (rebase) October 28, 2023 06:40
@d3c0d3d
Copy link

d3c0d3d commented Oct 28, 2023

Bad decision, removing something small and functional in favor of something large and bloated that not everyone was able to configure directly because there is no adequate documentation.

@expikr
Copy link
Contributor

expikr commented Oct 28, 2023

I'm in favor of removing it from the std.

But it shouldn't be deleted outright, nor should it just be moved to the std graveyard directly.

I'd like to suggest setting up dedicated projects for the individual DLLs,

i.e. ziglang.org/zig.user32 etc

The user32.zig presently being removed has two advantages over zigwin32:

  1. It is organized according to how you'd look it up on MSDN, whereas zigwin32's arbitrary namespacing makes it impossible to find what you need
  2. The handcrafted wrapping around the functions sets a good example of what idiomatic std interfaces looks like.

@squeek502
Copy link
Collaborator

squeek502 commented Oct 28, 2023

Just want to note:

removing something small and functional in favor of something large and bloated

I'd like to suggest setting up dedicated projects for the individual DLLs,

Creating a third-party library that provides small and functional / hand-crafted win32 bindings is something anyone is free to do, and indeed would be nice to have.

@@ -609,7 +609,7 @@ pub fn call_wWinMain() std.os.windows.INT {

// There's no (documented) way to get the nCmdShow parameter, so we're
// using this fairly standard default.
const nCmdShow = std.os.windows.user32.SW_SHOW;
const nCmdShow = 5;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not SW_SHOWDEFAULT? (10)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Certainly a better default than just SW_SHOW and okay for passing it to ShowWindow(), however nCmdShow in WinMain should contain the real value instead. You can get it like this:

const cmd_show = if (peb().ProcessParameters.WindowFlags & STARTF_USESHOWWINDOW != 0) peb().ProcessParameters.ShowWindowFlags else SW_SHOWDEFAULT;

Also I would be in favor of defining whatever constants are needed locally/leaving a comment, since just saying "5" or "10" isn't very useful.


Unrelated, but I think all the kernel32 usage in that file could be replaced by:

// GetModuleHandleW(null)
peb().ImageBaseAddress

// GetCommandLineW()
peb().ProcessParameters.CommandLine

// ExitProcess(status)
RtlExitUserProcess(status)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! All of these changes would make a great PR if you're up for it.

(I was also looking into this and found the PEB solution but missed the STARTF_USESHOWWINDOW wrinkle 👍)

@d3c0d3d
Copy link

d3c0d3d commented Oct 28, 2023

@expikr When I saw for the first time that Zig had the Windows API (or the minimum of it) in its core “std” in a simple and accessible way without depending on third-party packages, I thought it was wonderful, I come from Rust where the core is tiny There is no practically nothing and it is necessary to install outdated and poorly documented third-party libraries, I thought I was in paradise, now that they are removing it from Zig, for me it is a bad sign, for me something like "if you want to use this thing, turn around and good luck", that's the impression I have.

@expikr
Copy link
Contributor

expikr commented Oct 28, 2023

I think the bigger problem is that calling extern functions doesn't seem to be mentioned by any tutorials anywhere, but it's actually quite simple to do without requiring any complicated abstraction at all (which bloated libraries would mislead you into thinking it to be the case).

https://github.com/ziglang/www.ziglang.org/blob/master/assets/zig-code/samples/0-windows-msgbox.zig

const win = @import("std").os.windows;

extern "user32" fn MessageBoxA(?win.HWND, [*:0]const u8, [*:0]const u8, win.UINT) callconv(win.WINAPI) i32;

pub fn main() !void {
    _ = MessageBoxA(null, "world!", "Hello", 0);
}

I learned it by reading how they're declared in the user32.zig file, and I wouldn't have discovered it in the first place if it hadn't been in the std.

The presence of it being implemented in a single straightforward file provided an invaluable demonstration, it would be unwise to throw away the ladder after having already climbed it.

@squeek502
Copy link
Collaborator

squeek502 commented Oct 28, 2023

now that they are removing it from Zig, for me it is a bad sign

Note that this decision was made over 2 years ago in #4426. Here's a relevant comment from May 20, 2021 (#8776 (comment)):

Will merge this since it is a fix, but keep in mind that user32.zig will probably be entirely removed at some point.


"if you want to use this thing, turn around and good luck"

I see it more like "there's so much of this thing that people might want to use that it's probably better that the people actually using it take control" (note that user32.zig was entirely unused and untested by Zig itself). A third-party project that focused on providing user32 bindings would likely be able to provide well-tested and better overall bindings for the relevant use cases.

And, as @expikr points out, it's pretty easy to write up bindings for whatever Win32 function you need without any library at all (something I did a lot of myself for this).

@andrewrk andrewrk merged commit d817a3c into master Oct 28, 2023
9 of 10 checks passed
@andrewrk andrewrk deleted the user32-nai branch October 28, 2023 17:16
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.

5 participants