native constants improvements (NTSTATUS, LVS_*) etc

mitchcapper commented 1 year ago

I would say all of this is low priority (not that it isn't obvious) but potential ux improvements for the future:

Is your feature request related to a problem? Please describe. Right now one of three things can seem to happen with static constant values (aside from it refusing to generate it at all):

It gets put into a lovely enum ie: WINDOW_STYLE
It gets put as fields on a struct class ie: NTSTATUS
It is directly accessible off PInvoke (or sometimes in a deeper namespace)

Samples of each and current issues

Enum Style

Perfection, easy to work with not sure of any complications. If you try to import one of the flags on the enum it even warns you to just import the enum :) This mostly happens (I believe) if there is an API call that takes those flags as a parameter (and only those flags).

Fields on Struct

Works for checking a specific error, hr == NTSTATUS.STATUS_CACHE_PAGE_LOCKED not so good for trying to determine what a number translates to. Still get some autocomplete support as under a namespace/struct/class. Can work around this issue with some brute force work:

var type = typeof(NTSTATUS);
var matches = String.Join(", ", type.GetFields(all).Where(a => a.FieldType == type && status.Equals(a.GetValue(null))).Select(a=>$"{a.Name}"));
return new Win32Exception(status,$"NTSTATUS error: {matches}");

Win32Exception does some natively automatically based on the status code but is missing many (says unknown) that the above fixes.

Hanging off PInvoke directly

This is clearly the default if nothing else happens. This is rough as other than doing partial string matching with the struct style hack it is hard to work with. Of course this may be obvious, default case is default because it is the fallback.

Ideas

Clearly some of these values may be hard to automatically. Sometimes flags are separated out in the documentation ie: https://learn.microsoft.com/en-us/windows/win32/controls/list-view-window-styles although some are just all together. For many you have them at least all prefixed the same though. It would be nice if one could start having more configuration options in NativeMethods.json that could pattern match similar to the txt file. IE:

{
     "Prefix":"LVS_",
     "EnumName":"WINDOW_LIST_STYLES"
}

Moving to support a style like this (rather than something like preserveSigMethods which takes an array) it could allow other options like:

{
     "Prefix":"Windows.Win32.System.Threading",
     "allowMarshaling":false,
     "useIntPtrForComOutPointers":false
}

I am sure some of things are far more complex than global flags, but the json style would allow for easier (and maybe more natural) configuration classing.

I think the chance of getting things right for everyone is impossible but the more specific options certainly the more users can customize to fit their needs.

Finally, it would be good if c# namespaces could always be used in NativeMethods.json. CsWin32 pulls namespace data from somewhere, but it would make for easier importing of consts/structs that may otherwise require hoping to find the right prefix to pull them in. This has been brought up before and declined support plans. It is something that is probably a natural way to import and the information exists in CsWin32 (as it can map the items to those namespaces/classes/structs when the proper native name is given), but it is likely far easier to do a forward lookup than reverse.

AArnott commented 1 year ago

Your fields on a struct use case of trying a value to name lookup is interesting. CsWin32 theoretically could add a function to do a very fast match for this. It goes beyond the scope of delivering what the win32 api itself offers though, so this is a fine idea, but not committed to at this point.

As for enhancing the constants that hang off of PInvoke today, e.g. putting enums around them, that's work we do in the win32metadata repo. In fact that repo already has the means to collect a bunch of constants by prefix into an enum. If you have suggestions for enums, please file an issue over at https://github.com/microsoft/win32metadata.

Regarding your namespace suggestion, I'm not sure what your suggestion is. Are you asking for the ability to change the namespace that certain types are generated into? Are you asking for the ability to turn marshaling on and off on a per-namespace basis? FYI, every option we add to NativeMethods.json doubles the test matrix, and exhaustive testing of CsWin32 already takes a long time. Allowing per-namespace switches would make the test matrix so incredibly large we could never adequately test it, at least in our current design, so I don't see that happening at least in the near future and quite possibly ever.

mitchcapper commented 1 year ago

Regarding your namespace suggestion, I'm not sure what your suggestion is. Are you asking for the ability to change the namespace that certain types are generated into? Are you asking for the ability to turn marshaling on and off on a per-namespace basis?

Yes to be able to scope flags down, based on the complexities of testing though it is quite understandable that a minor convenience feature would have huge overhead. This is even still possible given the fact you could simply use two separate dlls with different sets of flags for cswin32 in the same parent application.

microsoft / CsWin32