On Wed, 30 Jun 2021 22:30:47 GMT, Kevin Rushforth <k...@openjdk.org> wrote:
>> 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 > > 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? well .. I usually like a short paragraph that succinctly says what it does as the first paragraph Anyway I've re-read all this and I prefer it as it is. > 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. But I don't say it twice. I say > The default value is an empty string, which is interpreted as unset, > equivalent to null, and > clear the value of this property by setting it to null or an empty string. which is somewhat different and makes i t very clear that either will work .. So I think this is all fine and flows as intended. > 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`. yep > 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). it is reset elsewhere not sure why a hyphen is here. > 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. ok split it ------------- PR: https://git.openjdk.java.net/jfx/pull/543