[Win32] Add support for options in Winspool notifications API #1301
Conversation
|
Thank you for coming back to this. I did not yet review, but have a request first: Please don't break the API. I would deprecate the old functions, add definitions for the new bindings and move documentation there. That way, old code keeps working, new code can benefit from the simplifcation and the cost is small. |
|
For anyone using Java 8 or later, their code will no longer compile if we add in deprecated versions of each method. @Deprecated
HANDLE FindFirstPrinterChangeNotification(
// _In_
HANDLE hPrinter,
int fdwFilter,
int fdwOptions,
// _In_opt_
LPVOID pPrinterNotifyOptions);
@Deprecated
boolean FindNextPrinterChangeNotification(
// _In_
HANDLE hChange,
// _Out_opt_
DWORDByReference pdwChange,
// _In_opt_
LPVOID pPrinterNotifyOptions,
// _Out_opt_
LPVOID ppPrinterNotifyInfo);If I change the source and target versions to 1.8 in Since Java 8 is the minimum supported version of Java by both Oracle and OpenJDK, I think it will be less likely to cause issues for users of the library to only have the updated version of each method - as they will only have been able to pass in |
|
It will only fail to compile if the arguments are // Get change notification handle for the printer
HANDLE chgObject = Winspool.INSTANCE
.FindFirstPrinterChangeNotification(phPrinter.getValue(),
Winspool.PRINTER_CHANGE_JOB, 0, (LPVOID) null);
if (chgObject != null) {
while (true) {
// Wait for the change notification
Kernel32.INSTANCE.WaitForSingleObject(chgObject,
WinBase.INFINITE);
DWORDByReference pdwChange = new DWORDByReference();
boolean fcnreturn = Winspool.INSTANCE
.FindNextPrinterChangeNotification(chgObject,
pdwChange, (LPVOID) null, (LPVOID) null);This code can be compiled against the new library and will create byte code that can be used with old version. I won't merge code, that breaks binary compatibility - while it is not nice to break source compatibility it is the lesser evil, as source code can be changed by definition. |
ianjoneill
force-pushed
the
f-winspool-notification-options
branch
from
4667480
to
9a1319a
Compare
|
Updated as per your request |
|
All I did was add in the overloaded methods - not sure why that caused a seemingly unrelated test to fail on Ubuntu Java 8 only. |
| if (count == 0) { | ||
| Count = count; | ||
| Version = (Integer) readField("Version"); | ||
| Flags = (Integer) readField("Flags"); |
There was a problem hiding this comment.
Please consider setting sData to the empty array - then you can always directly iterate over that array.
There was a problem hiding this comment.
Not sure what you mean - aData will become an empty array on line 782.
| /** | ||
| * Pointer to a buffer that contains the field's current data. | ||
| */ | ||
| public WinDef.PVOID pBuf; |
There was a problem hiding this comment.
I would bind this as a plain Pointer - I don't see the benefit to use a PVOID here. It just requires additional un/wrapping
Adds in the Java mappings for a number of structures required to allow specifying complex options to `Winspool`s notification APIs. Shout out to @matthiasblaesing for helping to fix the function definitions - more info here: https://groups.google.com/g/jna-users/c/FpWPY2c8bzY
ianjoneill
force-pushed
the
f-winspool-notification-options
branch
from
9a1319a
to
c4fca7a
Compare
|
So this time the Travis build flaked - again an unrelated test. |
|
Merged - thank you. I added a Changelog Entry for this. |
That one was fixed by #1300 |
Adds in the Java mappings for a number of structures required to
allow specifying complex options to
Winspools notification APIs.Shout out to @matthiasblaesing for helping to fix the function
definitions - more info here:
https://groups.google.com/g/jna-users/c/FpWPY2c8bzY