Skip to content

Added PRINTER_INFO_2 support and GetPrinter function#569

Merged
dblock merged 4 commits into
java-native-access:masterfrom
IvanRF:master
Jan 17, 2016
Merged

Added PRINTER_INFO_2 support and GetPrinter function#569
dblock merged 4 commits into
java-native-access:masterfrom
IvanRF:master

Conversation

@IvanRF

@IvanRF IvanRF commented Dec 20, 2015

Copy link
Copy Markdown
Contributor

Issue #568

@dblock

dblock commented Dec 21, 2015

Copy link
Copy Markdown
Member

Don't change the whole tabs vs. spaces thing, looks like your editor brought a bunch of extra changes here.

The new mappings need tests, please.

Update CHANGELOG.

@IvanRF

IvanRF commented Dec 21, 2015

Copy link
Copy Markdown
Contributor Author

I just updated the changelog. Also, I reduced the changes made by the editor (eclipse).

You can test PRINTER_INFO_2 with:

for (PRINTER_INFO_2 printerInfo : WinspoolUtil.getPrinterInfo2()) {
    System.out.println(printerInfo.pPrinterName + ": " + printerInfo.pDriverName);
}
System.out.println("---ALL---");
for (PRINTER_INFO_2 printerInfo : WinspoolUtil.getAllPrinterInfo2()) {
    System.out.println(printerInfo.pPrinterName + ": " + printerInfo.pDriverName);
}
System.out.println("---BY NAME---");
PRINTER_INFO_2 printerInfo = WinspoolUtil.getPrinterInfo2("Adobe PDF");
System.out.println(printerInfo.pPrinterName + ": " + printerInfo.pDriverName);

I just added the modified JNA classes to my project to test it. I don't know how to make an specific test inside JNA.

Comment thread CHANGES.md Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please make this look like the lines below. The class names are usually included with the full name like com.sun..., not everybody uses Windows.

@dblock

dblock commented Dec 21, 2015

Copy link
Copy Markdown
Member

You need actual tests for these methods. Even something basic that expects a certain return code or expects at least some data.

@dblock

dblock commented Dec 21, 2015

Copy link
Copy Markdown
Member

Please also squash multiple commits, thanks.

@IvanRF

IvanRF commented Dec 21, 2015

Copy link
Copy Markdown
Contributor Author

@dblock

dblock commented Dec 21, 2015

Copy link
Copy Markdown
Member

@IvanRF

IvanRF commented Dec 21, 2015

Copy link
Copy Markdown
Contributor Author

For the code in WinspoolUtil, I just use the other code as a base. Do you want me to add if (!function) throw new Win32Exception in every call?

@dblock

dblock commented Dec 22, 2015

Copy link
Copy Markdown
Member

WRT exception handling, it's possible old code didn't do this right, everything that can fail should be checked and raise an exception.

@IvanRF

IvanRF commented Dec 22, 2015

Copy link
Copy Markdown
Contributor Author

I added a check for OpenPrinter.

The return value for the first call to EnumPrinters is checked by

Winspool.INSTANCE.EnumPrinters(flags, null, 2, null, 0, pcbNeeded, pcReturned);
if (pcbNeeded.getValue() <= 0)
    return new PRINTER_INFO_2[0];

If an exception is added there, it will always throw it. I tested it.

Comment thread CHANGES.md Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The syntax of your line and the one below is different, it's - [@](...)., if you don't mind fixing please.

@dblock

dblock commented Dec 25, 2015

Copy link
Copy Markdown
Member

Makes sense WRT the Enum call, it's supposed to fail every time and return the number of things back. If you really want to be diligent you should be checking that it fails, then that the last error is something like insufficient buffer or something like that, but that's fine.

This still needs tests.

@IvanRF

IvanRF commented Jan 8, 2016

Copy link
Copy Markdown
Contributor Author

Fixed the syntax in CHANGED.md, I used the first entry as example but was wrong and I had followed that. I corrected both.

@dblock

dblock commented Jan 11, 2016

Copy link
Copy Markdown
Member

Still needs tests and a rebase.

@IvanRF

IvanRF commented Jan 16, 2016

Copy link
Copy Markdown
Contributor Author

@dblock rebase done.
Tests made by @mlfreeman2
I think we are finally done here!

@IvanRF
IvanRF force-pushed the master branch 2 times, most recently from 4eb408f to 5bbf48b Compare January 16, 2016 19:14
@dblock

dblock commented Jan 17, 2016

Copy link
Copy Markdown
Member

i am cool with this, merging.

dblock added a commit that referenced this pull request Jan 17, 2016
Added PRINTER_INFO_2 support and GetPrinter function
@dblock
dblock merged commit 2f0c4c3 into java-native-access:master Jan 17, 2016
@IvanRF

IvanRF commented Jan 17, 2016

Copy link
Copy Markdown
Contributor Author

Thanks! 👍

mstyura pushed a commit to mstyura/jna that referenced this pull request Sep 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants