Add definitions for Wincon.GetConsoleScreenBufferInfo() #1190

rednoah · 2020-05-10T19:16:34Z

Add the missing definitions for the GetConsoleScreenBufferInfo function of the Windows Console API:
https://docs.microsoft.com/en-us/windows/console/getconsolescreenbufferinfo

dbwiddis

Added a few comments inline. Can you add a test case to Kernel32ConsoleTest exercising this method?

dbwiddis · 2020-05-10T19:27:52Z

contrib/platform/src/com/sun/jna/platform/win32/WinDef.java

+
+        @Override
+        public String toString() {
+            return String.format("COORD[%s,%s]", X, Y);


X and Y are primitives, so should the %s be %d here?

When I wrote this, I thought surely Java was more efficient knowing it had an integer value here. In fact, the way you have it simply invokes Short.toString(X) while the %d version goes through a bunch of conversions to BigInteger, extracting a long from that, and finally calling Long.toString(), although I don't know if any of those steps are eventually optimized out...

My personal rule is to avoid %d because you will unexpectedly get different results depending on Locale.getDefault().

e.g.

Locale.setDefault(new Locale("ar")) def s = String.format("%d,%d,%d", 1, 2, 3) System.out.println(s) // prints "١,٢,٣" (and not "1,2,3" as one would expect)

Don't ask me how I know this. Those are many ours of my life I won't get back. ٣ doesn't match \d and can't be parsed as number, and Windows CMD won't print it because it's not ASCII either, and the issue will only affect users in certain with uncommon rare locales so you won't ever be able to reproduce the issue with a debugger on a typical developer machine that's set on en_US.

Sounds reasonable!

dbwiddis · 2020-05-10T19:30:36Z

contrib/platform/src/com/sun/jna/platform/win32/WinDef.java

+
+        @Override
+        public String toString() {
+            return String.format("SMALL_RECT[%s,%s,%s,%s]", Left, Top, Right, Bottom);


Same comment here on %d vs %s. Also just my opinion, but while (X,Y) is standard for coordinates, I'm not sure there's a generally accepted standard order of values for a "rectangle". I've seen (left,top,width,height) for example, so this would be more clear with L=%d,T=%d,R=%d,B=%d or possibly combining left/top and right/bottom as a coordinate pair (%d,%d)(%d,%d).

I just added toString() for the sake of completeness. I don't actually use them in my code. I'm happy to make any changes you see fit.

I'm just offering suggestions!

Sure thing. I switched to () because the other classes use that as well.

rednoah · 2020-05-11T10:01:22Z

Is it ok to just have a dummy test in Kernel32ConsoleTest that just checks if the method is called and fails?

Because we'll need a CMD window (i.e. System.console() != null) for this method to return something meaningful (e.g. won't work when running via Eclipse).

rednoah · 2020-05-11T11:14:46Z

I've added a @Test @Ignore inline with another test that is disabled because it only works when running with a CMD window.

…teractive console context

matthiasblaesing

Thank you looks sane to me. I left one inline comment about a potential optimisation.

Before merging, please add an entry to the features section of the CHANGES.md file. Please follow the examples set in the existing entries. Thank you.

matthiasblaesing · 2020-05-12T17:41:27Z

contrib/platform/src/com/sun/jna/platform/win32/WinDef.java

+
+        public COORD dwSize;
+        public COORD dwCursorPosition;
+        public WORD wAttributes;


COORD and SMALL_RECT both map to java shorts. It would be an option to also map wAttributes to java short. If I read the documentation correctly it is a set of shorts ORed together, so the sign is not relevant.

I thought similar when I looked at it, but decided the plain structure interpretation (dwSize.X and dwSize.Y) would be preferred over an int with bitshifting/bitmasking getters to access the underlying values. About the only thing I might change in this mapping is that wAttributes as a short would more directly map to the WINAPI enum values it represents.

Er... that's what you said. I misread it the first time.

rednoah · 2020-05-13T06:57:19Z

I'm adding bindings for a few additional Windows Console API functions. Please don't merge anything else, so we can all do it in one pull later on.

* ReadConsoleInput * GetNumberOfConsoleInputEvents * WriteConsole

rednoah · 2020-05-13T14:00:19Z

Done. All that just so we can capture RESIZE events and print Emoji. 👯

…b/platform/src/com/sun/jna/platform/win32/WinDef.java:1852: 'case' child has incorrect indentation level 12, expected level should be 16. [Indentation]

matthiasblaesing

I only had a quick glance, but I noticed, that WORD could be mapped to a java short, DWORD can be mapped to a int and BOOL could be mapped to boolean. DWORD and WORD are only interesting IMHO when the unsigned nature is important and trumps the easier handling of integer in java. The corresponding ByReference types exist and I would use these.

rednoah · 2020-05-13T15:57:19Z

I saw lots of code in the platform package to use those WinDef classes. It does help with making the JNA definition look exactly the same as the Windows API references.

matthiasblaesing · 2020-05-13T17:53:00Z

Yes, but it also makes it create more garbadge (more wrapper classes) and harder to use as you need to wrap each number. This is also not consistent, the COORD and SMALL_RECT structs are both mapped to java shorts.

As said, there are reasons to prefer the wrappers, but I don't see many. For an enum like value like EventType for example, that is created from constants or ORed from constants you don't gain anything.

rednoah · 2020-05-14T03:39:25Z

Reasonable enough. I'll switch to native types and run a few tests.

rednoah · 2020-05-15T12:08:17Z

/home/travis/build/java-native-access/jna/build.xml:402: javac doesn't support the "nativeheaderdir" attribute seems to be unrelated to my changes.

matthiasblaesing · 2020-05-15T17:46:00Z

Please rebase this PR onto current master, @dbwiddis fixed a stability issue, that was caused by wrong assumptions about the apache mirror system.

Fix statvfs filesystem - it is unstable in docker environments

Remove invalid GetTcpStatistics test

As of 2020-05-13, there are two versions of Apache Ant 1.9.x binaries available for download. The regexp parsing incorrectly resolves to a string containing both versions, separated by a newline character, and the download fails. This change limits the grep to only the last line, so the latest 1.9.x version will be downloaded.

Only fetch the latest ant version in Travis script

…to primarygroup

Add getTokenPrimaryGroup to Advapi32Util

Closes: #1182

rednoah · 2020-05-16T07:20:24Z

Mmhhh. Now these changes appear in my pull request as well...

dbwiddis · 2020-05-16T07:32:06Z

That's an annoying "feature" of github. The steps here should fix it (basically: add a dummy file, commit, push, rebase to previous commit before you added the file, force push).

rednoah · 2020-05-16T08:42:23Z

Didn't work. Nevermind. I'll just fork again and apply a patch and create a new clean pull request.

rednoah · 2020-05-16T08:53:26Z

👉 #1194

Add definitions for Wincon.GetConsoleScreenBufferInfo()

f4bb4a7

dbwiddis reviewed May 10, 2020

View reviewed changes

rednoah added 2 commits May 11, 2020 18:10

Added testGetConsoleScreenBufferInfo()

2757af3

Change toString() to be more in line with other toString() methods

b3c10e8

Activate test case and make it work for both interactivate and non-in…

aad550b

…teractive console context

matthiasblaesing reviewed May 12, 2020

View reviewed changes

Added Windows Console API bindings:

1d76826

* ReadConsoleInput * GetNumberOfConsoleInputEvents * WriteConsole

[checkstyle] [ERROR] /home/travis/build/java-native-access/jna/contri…

1dfdd22

…b/platform/src/com/sun/jna/platform/win32/WinDef.java:1852: 'case' child has incorrect indentation level 12, expected level should be 16. [Indentation]

matthiasblaesing reviewed May 13, 2020

View reviewed changes

Prefer native types over WinDef types

92a6d5a

matthiasblaesing and others added 11 commits May 16, 2020 14:14

Fix statvfs filesystem - it is unstable in docker environments

061b826

Merge pull request #1188 from matthiasblaesing/unittest-docker

a6a2b43

Fix statvfs filesystem - it is unstable in docker environments

Remove invalid GetTcpStatistics test

781ca72

Merge pull request #1189 from dbwiddis/tcptest

db60d47

Remove invalid GetTcpStatistics test

Add getTokenPrimaryGroup to Advapi32Util

889f5dd

Merge pull request #1193 from dbwiddis/patch-1

57b5dc8

Only fetch the latest ant version in Travis script

Merge branch 'master' of https://github.com/java-native-access/jna in…

f9f835c

…to primarygroup

Merge pull request #1191 from dbwiddis/primarygroup

c6c7a48

Add getTokenPrimaryGroup to Advapi32Util

Add toString to classes extending ByReference

54f990a

Merge branch 'pr-1182'

8fd977f

Closes: #1182

ByReferencePlatformToStringTest.java was not correctly merged

c6f2e5a

rednoah closed this May 16, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add definitions for Wincon.GetConsoleScreenBufferInfo() #1190

Add definitions for Wincon.GetConsoleScreenBufferInfo() #1190

rednoah commented May 10, 2020 •

edited

dbwiddis left a comment

dbwiddis May 10, 2020

dbwiddis May 10, 2020

rednoah May 11, 2020

rednoah May 11, 2020 •

edited

dbwiddis May 11, 2020

dbwiddis May 10, 2020

rednoah May 11, 2020

dbwiddis May 11, 2020

rednoah May 12, 2020

rednoah commented May 11, 2020

rednoah commented May 11, 2020 •

edited

matthiasblaesing left a comment

matthiasblaesing May 12, 2020

dbwiddis May 12, 2020

dbwiddis May 12, 2020

rednoah commented May 13, 2020

rednoah commented May 13, 2020

matthiasblaesing left a comment

rednoah commented May 13, 2020

matthiasblaesing commented May 13, 2020

rednoah commented May 14, 2020

rednoah commented May 15, 2020

matthiasblaesing commented May 15, 2020

rednoah commented May 16, 2020

dbwiddis commented May 16, 2020 •

edited

rednoah commented May 16, 2020

rednoah commented May 16, 2020

Add definitions for Wincon.GetConsoleScreenBufferInfo() #1190

Add definitions for Wincon.GetConsoleScreenBufferInfo() #1190

Conversation

rednoah commented May 10, 2020 • edited

dbwiddis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rednoah May 11, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rednoah commented May 11, 2020

rednoah commented May 11, 2020 • edited

matthiasblaesing left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rednoah commented May 13, 2020

rednoah commented May 13, 2020

matthiasblaesing left a comment

Choose a reason for hiding this comment

rednoah commented May 13, 2020

matthiasblaesing commented May 13, 2020

rednoah commented May 14, 2020

rednoah commented May 15, 2020

matthiasblaesing commented May 15, 2020

rednoah commented May 16, 2020

dbwiddis commented May 16, 2020 • edited

rednoah commented May 16, 2020

rednoah commented May 16, 2020

rednoah commented May 10, 2020 •

edited

rednoah May 11, 2020 •

edited

rednoah commented May 11, 2020 •

edited

dbwiddis commented May 16, 2020 •

edited