jdk
jdk copied to clipboard
8305457: Implement java.io.IO
Please review this PR which introduces the java.io.IO
top-level class and three methods to java.io.Console
for Implicitly Declared Classes and Instance Main Methods (Third Preview).
This PR has been obtained as git merge --squash
of a now obsolete draft PR.
Progress
- [x] Change must be properly reviewed (1 review required, with at least 1 Reviewer)
- [x] Change must not contain extraneous whitespace
- [x] Commit message must refer to an issue
- [ ] Change requires CSR request JDK-8331610 to be approved
Issues
- JDK-8305457: Implement java.io.IO (Enhancement - P3)
- JDK-8331610: Implement java.io.IO (CSR)
Reviewers
- Stuart Marks (@stuart-marks - Reviewer)
- Jaikiran Pai (@jaikiran - Reviewer)
Reviewing
Using git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/19112/head:pull/19112
$ git checkout pull/19112
Update a local copy of the PR:
$ git checkout pull/19112
$ git pull https://git.openjdk.org/jdk.git pull/19112/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 19112
View PR using the GUI difftool:
$ git pr show -t 19112
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/19112.diff
Webrev
:wave: Welcome back prappo! A progress list of the required criteria for merging this PR into master
will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.
@pavelrappo This change now passes all automated pre-integration checks.
ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.
After integration, the commit message for the final commit will be:
8305457: Implement java.io.IO
Reviewed-by: naoto, smarks, jpai, jlahoda
You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.
At the time when this comment was updated there had been 3 new commits pushed to the master
branch:
- 6a35311468222f9335b43d548df2ecb80746b389: 8241550: [macOS] SSLSocketImpl/ReuseAddr.java failed due to "BindException: Address already in use"
- f16265d69b09640b972b7494ad57158dbdc426bb: 8332226: "Invalid package name:" from source launcher
- 5a2ba952b120394d7cc0d0890619780c1c27a078: 8325841: Remove unused references to vmSymbols.hpp
Please see this link for an up-to-date comparison between the source branch of this pull request and the master
branch.
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.
➡️ To integrate this PR with the above commit message to the master
branch, type /integrate in a new comment.
@pavelrappo The following labels will be automatically applied to this pull request:
-
core-libs
-
kulla
When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command.
Webrevs
- 13: Full (5edf686d)
- 12: Full (258ce133)
- 11: Full - Incremental (e252c5d8)
- 10: Full - Incremental (719560f6)
- 09: Full (809e98e0)
- 08: Full - Incremental (17100ab8)
- 07: Full - Incremental (43a95732)
- 06: Full - Incremental (1bf7a4cd)
- 05: Full - Incremental (46a7af1f)
- 04: Full - Incremental (401d8d25)
- 03: Full - Incremental (80eacf8c)
- 02: Full - Incremental (73a20a7c)
- 01: Full - Incremental (60050834)
- 00: Full (ef888fd8)
Otherwise looks good.
Mailing list message from Olexandr Rotan on core-libs-dev:
Has it been considered to add a readKey() method to IO class? In my experience, it is pretty commonly used by beginners when they write things like console games (snake, catcher game etc.). I am aware there is System.in.read() method, but since we decided to promote some methods to separate methods in IO class, maybe readKey() or simple read() should also be considered?
On Wed, May 8, 2024 at 4:16?PM Pavel Rappo <prappo at openjdk.org> wrote:
-------------- next part -------------- An HTML attachment was scrubbed... URL: <https://mail.openjdk.org/pipermail/core-libs-dev/attachments/20240508/28c90cb6/attachment-0001.htm>
Mailing list message from Pavel Rappo on core-libs-dev:
I don't remember it being considered, but I think it falls outside the scope of JEP 477 (emphasis mine):
We achieve this effect by declaring a new top-level class in the java.io package named, simply, IO. It declares the above three static methods for **textual** I/O with the console, and nothing else.
java.io.IO is closely related to java.io.Console, which provides access to a terminal in a more or less canonical mode to read and write _strings_. While it might be useful for Console to separately provide access to a terminal in the raw mode, I'm not sure how often developers, let alone on-ramp developers, need it. In my experience, programming games, such as those you mentioned, is better introduced with GUI, where they have java.awt.event.KeyListener.
-Pavel
(Minor). at least within the context of this PR, tags like
@param
and@throws
are stylistically inconsistent as to whether they begin with a capital letter and end with a period. (I have not checked for local consistency within the affected files.)
I know, thanks. That was on purpose to mimic the prevailing style of Console
, just like you assumed in parentheses. Note, like-named methods in IO
are styled differently.
Latest changes look good.
@pavelrappo this pull request can not be integrated into master
due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:
git checkout 8305457-Implement-java.io.IO
git fetch https://git.openjdk.org/jdk.git master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push
/integrate
Going to push as commit c099f14f07260713229cffbe7d23aa8305415a67.
Since your change was applied there have been 3 commits pushed to the master
branch:
- 6a35311468222f9335b43d548df2ecb80746b389: 8241550: [macOS] SSLSocketImpl/ReuseAddr.java failed due to "BindException: Address already in use"
- f16265d69b09640b972b7494ad57158dbdc426bb: 8332226: "Invalid package name:" from source launcher
- 5a2ba952b120394d7cc0d0890619780c1c27a078: 8325841: Remove unused references to vmSymbols.hpp
Your commit was automatically rebased without conflicts.
@pavelrappo Pushed as commit c099f14f07260713229cffbe7d23aa8305415a67.
:bulb: You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.