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

Win32 Console API: GetStdHandle, WriteConsole, GetCurrentConsoleFont #251

Closed
wants to merge 19 commits into from

Conversation

tmyroadctfig
Copy link
Contributor

Added:

  • GetStdHandle()
  • WriteConsole()
  • GetCurrentConsoleFont()

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.

/**
* The index of the font in the system's console font table.
*/
public WinDef.DWORD nFont;
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

@dblock
Copy link
Member

dblock commented Jul 23, 2013

Looks great. Please check out @twall's comment and also update CHANGELOG. Thank you!

@tmyroadctfig
Copy link
Contributor Author

Those changes are in now.

Cheers, Luke

@dblock
Copy link
Member

dblock commented Jul 30, 2013

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.

@tmyroadctfig
Copy link
Contributor Author

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.

@dblock
Copy link
Member

dblock commented Jul 30, 2013

I thought that too, but I also tried the equivalent of this, which creates a console if it doesn't exist:

 AllocConsole();
 stdout = GetStdHandle( STD_OUTPUT_HANDLE );
 WriteConsole(stdout, "Test", 4, &count, NULL );
 FreeConsole(); 

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.

@dblock
Copy link
Member

dblock commented Jul 30, 2013

Btw, I think there's something fishy with the signed/unsigned handle values, I believe the result from GetStdHandle is indeed invalid with this code.

@dblock
Copy link
Member

dblock commented Aug 2, 2013

Confirming that these tests fail command-line, too.

@dblock
Copy link
Member

dblock commented Sep 25, 2013

Bump.

Conflicts:
	CHANGES.md
	contrib/platform/src/com/sun/jna/platform/win32/Kernel32.java
	contrib/platform/test/com/sun/jna/platform/win32/Kernel32Test.java
@tmyroadctfig
Copy link
Contributor Author

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?

public static final int FILE_DEVICE_ACPI = 0x00000032;
public static final int FILE_DEVICE_BATTERY = 0x00000029;
public static final int FILE_DEVICE_BEEP = 0x00000001;
public static final int FILE_DEVICE_BUS_EXTENDER = 0x0000002a;
Copy link
Contributor

Choose a reason for hiding this comment

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

  • This should be an interface (as indicated also by the Javadoc)
  • Please drop the public static final as this is the default for interfaces

@lgoldstein
Copy link
Contributor

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

@tmyroadctfig
Copy link
Contributor Author

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.

public PROCESSOR_RELATIONSHIP processor;

public SYSTEM_LOGICAL_PROCESSOR_INFORMATION_EX()
{}
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

@lgoldstein
Copy link
Contributor

Besides the specific (minor) comments:

  • Add a line in CHANGES.md with this pull request details (see other lines)
  • Squash everything to a single commit (you currently have 13...) so we can merge this as a one "atomic" change

…AP_EJECTSUPPORTED and SPDRP_FRIENDLYNAME. Added SYSTEM_LOGICAL_PROCESSOR_INFORMATION_EX, MEMORY_BASIC_INFORMATION64, STORAGE_DEVICE_DESCRIPTOR, DISK_GEOMETRY_EX, and related structures
@ForNeVeR
Copy link

ForNeVeR commented Sep 17, 2016

The title of this PR is misleading: there're no WriteConsole in the current JNA code base neither in this PR.

@tmyroadctfig
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants