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

Should GUID_NULL really be in windows::Win32::Media::KernelStreaming? #1772

Open
Enyium opened this issue Dec 21, 2023 · 4 comments
Open

Should GUID_NULL really be in windows::Win32::Media::KernelStreaming? #1772

Enyium opened this issue Dec 21, 2023 · 4 comments
Labels
enhancement New feature or request

Comments

@Enyium
Copy link

Enyium commented Dec 21, 2023

Suggestion

Shell_NotifyIconGetRect() demands GUID_NULL for its struct parameter NOTIFYICONIDENTIFIER, which has nothing to to with windows::Win32::Media::KernelStreaming that GUID_NULL currently resides in.

Should a user wanting to use the shell API really have to add a dependency to this module? GUID is even from windows::core!

@Enyium Enyium added the enhancement New feature or request label Dec 21, 2023
@tim-weis
Copy link
Contributor

GUID::zeroed() returns the same value as GUID_NULL. You do not need to enable the "Win32_Media_KernelStreaming" feature to use this value.

@Enyium
Copy link
Author

Enyium commented Dec 21, 2023

I didn't expect such a specific method, because other types always require use of default() to get an instance with zeroed memory.

This is what I mean with too large API inconsistency. When GUID_NULL exists, GUID::null() would be justified. And then, many other types would also need it, like HWND::null(), HMENU::null() etc. This, in turn, would IMO justify the trait Null that could be used for these types (and also offer is_null()).

You should be able to better get to know and predict the API, instead of being surprised by it again and again.

I also don't really like the statement that writing default() makes on types like HICON etc. For types like POINT and SIZE, depending on the context (like CreateWindowEx() with its CW_USEDEFAULT), POINT::default() and SIZE::default() can be misleading in an abstraction. And they also don't have zeroed().


Regarding GUID_NULL, I think it's still debatable whether something documented as usable for a shell function should only be available in such a different module.

@riverar
Copy link
Collaborator

riverar commented Dec 21, 2023

This is what I mean with too large API inconsistency. When GUID_NULL exists, GUID::null() would be justified. [...]

Disagree. That would not accurately depict {00000000-0000-0000-0000-000000000000}, a valid GUID.

Regarding GUID_NULL, I think it's still debatable whether something documented as usable for a shell function should only be available in such a different module.

Fair feedback, might be something we can move around. I'll forward this issue to the win32metadata repository.

@riverar riverar transferred this issue from microsoft/windows-rs Dec 21, 2023
@Enyium
Copy link
Author

Enyium commented Dec 21, 2023

Disagree. That would not accurately depict {00000000-0000-0000-0000-000000000000}, a valid GUID.

What's the difference? std::ptr::null() is also implemented as a function. But you could also define it as an associated constant by the trait:

#[repr(transparent)]
pub struct HWND(pub isize);

pub trait Null {
    const NULL: Self;
    
    fn is_null(&self) -> bool;
}

impl Null for HWND {
    const NULL: Self = HWND(0);
    
    fn is_null(&self) -> bool {
        self.0 == 0
    }
}

fn main() {
    let hwnd = HWND::NULL;
    println!("{}", hwnd.is_null());
}

Do you also object to GUID::NULL, even though there's GUID_NULL?

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

No branches or pull requests

3 participants