[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-openjfx-dev
Subject:    Re: RFR: 8223717: javafx printing: Support Specifying Print to File in the API [v5]
From:       Kevin Rushforth <kcr () openjdk ! java ! net>
Date:       2021-06-30 22:42:08
Message-ID: o83V57R11Q6YSPjKIB_usK_IQc0vxUf7ug9JrJ1HGcs=.485a74bb-695a-4f8f-8678-9f438bf2b248 () github ! com
[Download RAW message or body]

On Wed, 30 Jun 2021 00:48:25 GMT, Phil Race <prr@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


[prev in list] [next in list] [prev in thread] [next in thread]