Win32 Console API: GetStdHandle, WriteConsole, GetCurrentConsoleFont#251
Win32 Console API: GetStdHandle, WriteConsole, GetCurrentConsoleFont#251tmyroadctfig wants to merge 19 commits intojava-native-access:masterfrom
Conversation
There was a problem hiding this comment.
As a matter of style, make this library interface extend WinDef so that any part of the interface or its derivations can reference DWORD instead of WinDef.DWORD. DWORD is common enough to warrant dropping the prefix.
|
Looks great. Please check out @twall's comment and also update CHANGELOG. Thank you! |
Code review changes.
|
Those changes are in now. Cheers, Luke |
|
I checked these out. On my Windows 7, 64-bit the three new tests fail with "invalid handle". I didn't see anything immediately wrong with the code, and wrote the same thing in C++, which worked, just to see if there's something different on my machine specifically. |
|
I think that's because the console font will only be defined if there is one of those black cmd.exe windows. The tests failed for me under IDEA and I was thinking that was the reason. Did you run the tests via ant inside a command prompt? I haven't check that case myself actually. |
|
I thought that too, but I also tried the equivalent of this, which creates a console if it doesn't exist: Didn't see whether the command line version succeeds, but I suspect it doesn't. I'll have a Windows PC in front of me today or tomorrow and can try this if you haven't gotten to it yet. |
|
Btw, I think there's something fishy with the signed/unsigned handle values, I believe the result from |
|
Confirming that these tests fail command-line, too. |
|
Bump. |
Conflicts: CHANGES.md contrib/platform/src/com/sun/jna/platform/win32/Kernel32.java contrib/platform/test/com/sun/jna/platform/win32/Kernel32Test.java
# Conflicts: # contrib/platform/src/com/sun/jna/platform/win32/Wincon.java
|
Sorry this got forgotten. I looks like most of my changes got copied in via some other change. Can you please review this newly added structures for WinNT, Winioctl, etc? |
There was a problem hiding this comment.
- This should be an interface (as indicated also by the Javadoc)
- Please drop the public static final as this is the default for interfaces
|
I recommend waiting a little while until pull request #574 is merged and then change the getFieldsOrder method(s) of the various new Structure(s) to use the createFieldsOrder method, as demonstrated by the pull request changes |
# Conflicts: # contrib/platform/src/com/sun/jna/platform/win32/Winioctl.java
|
OK, I've updated the code here to switch over to match the changes in #574 as suggested. Hopefully this is ready to review again. |
There was a problem hiding this comment.
Don't leave this empty - better to add the call to super() - even though it is implicit, as this enables placing a debug breakpoint in the constructor.
|
Besides the specific (minor) comments:
|
…AP_EJECTSUPPORTED and SPDRP_FRIENDLYNAME. Added SYSTEM_LOGICAL_PROCESSOR_INFORMATION_EX, MEMORY_BASIC_INFORMATION64, STORAGE_DEVICE_DESCRIPTOR, DISK_GEOMETRY_EX, and related structures
|
The title of this PR is misleading: there're no |
|
The title is a bit out of date and I probably won't have any time in the near future to tidy up the code ready to merge. Closing this off for now. |
Added:
I've added in a test for WriteConsole and GetCurrentConsoleFont but they don't pass from IDEA. I'm assume that's because there is no console window displayed.