RFR: 8223717: javafx printing: Support Specifying Print to File in the API [v5]

[Kevin Rushforth]

On Wed, 30 Jun 2021 00:48:25 GMT, Phil Race <prr at openjdk.org> wrote:

>> This enhancement adds the String property outputFileProperty() to the JobSettings class.
>> The value should be a string that references a local file encoded as a URL.
>> If this is non-null and set to a location that the user has permission to write to,
>> then the printer output will be spooled there instead of the printer, so long as the platform printing system supports this.
>> The user can of course also set a print-to-file destination in the platform printer dialogs which may over-ride what the application set. But now the application can also see what it was set to, and cancel or alter it if necessary.
>> 
>> A simple manual test is provided, manual mainly because the few real printing functional tests are all manual as they are only useful if run with a printer configured.
>
> Phil Race has updated the pull request incrementally with three additional commits since the last revision:
> 
>  - 8223717: javafx printing: Support Specifying Print to File in the API
>  - 8223717: javafx printing: Support Specifying Print to File in the API
>  - 8223717: javafx printing: Support Specifying Print to File in the API

I've added my final comments on the API docs. Some of them are pretty nit-picky, so take them as suggestions.

modules/javafx.graphics/src/main/java/javafx/print/JobSettings.java line 481:

> 479:      * such as Postscript or PDF, and the application intends to distribute
> 480:      * the result instead of printing it, or for some other reason the
> 481:      * application does not want physical media (paper) emitted by the printer.

Very minor: maybe consider combining the first three paragraphs into a single paragraph?

modules/javafx.graphics/src/main/java/javafx/print/JobSettings.java line 486:

> 484:      * equivalent to null, which means output is sent to the printer.
> 485:      * So in order to reset to print to the printer, clear the value of
> 486:      * this property by setting it to null or an empty string.

This doesn't flow as well as it could. I think you only need to mention once that `null` is the same as an empty string, and then you can just say "empty string". Maybe something like this?


The default value is an empty string, which means output is sent to the printer. So in order to reset
to print to the printer, clear the value of this property by setting it to an empty string. A value
of {@code null} is treated as an empty string.

modules/javafx.graphics/src/main/java/javafx/print/JobSettings.java line 489:

> 487:      * <p>
> 488:      * Additionally if the application displays a printer dialog which allows
> 489:      * the user to specify a file destination including altering an application

Minor: I think there should be a comma between `destination` and `including`.

modules/javafx.graphics/src/main/java/javafx/print/JobSettings.java line 491:

> 489:      * the user to specify a file destination including altering an application
> 490:      * specified file destination, the value of this property will reflect that
> 491:      * user-specified choice, including clearing it to re-set to print to

`reset` is one word (no need to hyphenate).

modules/javafx.graphics/src/main/java/javafx/print/JobSettings.java line 500:

> 498:      * a user writable file, when printing the results are platform-dependent, including
> 499:      * replacement with a default output file location, printing to the printer instead,
> 500:      * or a platform printing error.

This sentence is a bit awkward and hard to parse. Maybe break it into two sentences? Perhaps something like this:


If the specified name specifies a non-existent path, or does not specify a user writable file, the
results when printing are platform-dependent. Possible behavior might include replacement with
a default output file location, printing to the printer instead, or a platform printing error.

-------------

PR: https://git.openjdk.java.net/jfx/pull/543