jdk icon indicating copy to clipboard operation
jdk copied to clipboard

Add new method JavaShellToolBuilder.windowSize().

Open archiecobbs opened this issue 1 year ago • 3 comments

When launching JShell programmatically (i.e., from a Java program instead of the command line) for an interactive session, it's not currently possible to inform JShell what the terminal window's dimensions are. As a result, JShell defaults to 80x24 and line editing becomes almost impossible because of the scrambled screen contents (unless you happen to be logged in remotely from an Apple 2).

This patch adds a new method JavaShellToolBuilder.windowSize() which allows passing a "hint" for the number of rows & columns.


Progress

  • [ ] Change must be properly reviewed (1 review required, with at least 1 Reviewer)
  • [x] Change must not contain extraneous whitespace
  • [ ] Commit message must refer to an issue

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/19226/head:pull/19226
$ git checkout pull/19226

Update a local copy of the PR:
$ git checkout pull/19226
$ git pull https://git.openjdk.org/jdk.git pull/19226/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 19226

View PR using the GUI difftool:
$ git pr show -t 19226

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/19226.diff

archiecobbs avatar May 14 '24 02:05 archiecobbs

:wave: Welcome back acobbs! 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.

bridgekeeper[bot] avatar May 14 '24 02:05 bridgekeeper[bot]

@archiecobbs 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:

8332314: Add window size configuration option to JavaShellToolBuilder interface

Reviewed-by: 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 48 new commits pushed to the master branch:

  • 001d6860199436c5fb14bd681d640d462b472015: 8332587: RISC-V: secondary_super_cache does not scale well
  • 5cad0b4df7f5ccb6d462dc948c2ea5ad5da6e2ed: 8322708: Global HTML attributes are not allowed
  • 642084629a9a793a055cba8a950fdb61b7450093: 8334396: RISC-V: verify perf of ReverseBytesI/L
  • c6f3bf4bd61405c2ed374b15ef82cc987f52cd52: 8334026: Provide a diagnostic PrintMemoryMapAtExit switch on Linux
  • cabd1046d08865f122663d18708d40e5c885c1c3: 8334164: The fix for JDK-8322811 should use _filename.is_set() rather than strcmp()
  • d7dad50af5df356089101ca440fca5232fadb81e: 8334544: C2: wrong control assigned in PhaseIdealLoop::clone_assertion_predicate_for_unswitched_loops()
  • ff30240926224b2f98e173bcd606c157af788919: 8334239: Introduce macro for ubsan method/function exclusions
  • 2d4185f4f1def7c32d1a556521e26ec656234220: 8332717: ZGC: Division by zero in heuristics
  • fad6644eabbad6b6d3472206d9db946408aca612: 8333754: Add a Test against ECDSA and ECDH NIST Test vector
  • b211929e05c0acdf7343c3edd025749d573c67b3: 8334570: Problem list gc/TestAlwaysPreTouchBehavior.java
  • ... and 38 more: https://git.openjdk.org/jdk/compare/8464ce6db5cbd5d50ac2a2bcba905b7255f510f5...master

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.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@lahodaj) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

openjdk[bot] avatar May 14 '24 02:05 openjdk[bot]

@archiecobbs The following label will be automatically applied to this pull request:

  • kulla

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

openjdk[bot] avatar May 14 '24 02:05 openjdk[bot]

Webrevs

mlbridge[bot] avatar May 15 '24 17:05 mlbridge[bot]

@lahodaj (or anyone else) - Any comments on this idea?

Presumably this requires a CSR, but I wanted to get a quick sanity check before proceeding further.

Thanks.

archiecobbs avatar May 30 '24 00:05 archiecobbs

Sorry for belated reply. This makes sense to me.

lahodaj avatar May 31 '24 06:05 lahodaj

Awesome, thanks. I'll work on a CSR.

archiecobbs avatar May 31 '24 18:05 archiecobbs

/csr

archiecobbs avatar May 31 '24 18:05 archiecobbs

@archiecobbs has indicated that a compatibility and specification (CSR) request is needed for this pull request.

@archiecobbs please create a CSR request for issue JDK-8332314 with the correct fix version. This pull request cannot be integrated until the CSR request is approved.

openjdk[bot] avatar May 31 '24 18:05 openjdk[bot]

Hi @lahodaj, if you get a chance, please review the CSR JDK-8333379 or recommend someone else who might be a good reviewer. Thanks.

archiecobbs avatar Jun 03 '24 22:06 archiecobbs

Still looking for a reviewer for this (CSR already approved)... thanks.

archiecobbs avatar Jun 12 '24 19:06 archiecobbs

Unfortunately we have to defer this to release 24 now, and seems only @lahodaj know about jshell well enough for a review.

liach avatar Jun 12 '24 20:06 liach

/integrate

archiecobbs avatar Jun 13 '24 16:06 archiecobbs

@archiecobbs Your change (at version 39fb0972d64a02397a057d7a49b5d03652b50ebb) is now ready to be sponsored by a Committer.

openjdk[bot] avatar Jun 13 '24 16:06 openjdk[bot]

Thanks for the review.

archiecobbs avatar Jun 20 '24 14:06 archiecobbs

/integrate

archiecobbs avatar Jun 20 '24 14:06 archiecobbs

@archiecobbs Your change (at version 1769df6dff86844753b51f570e918220b1e30f7c) is now ready to be sponsored by a Committer.

openjdk[bot] avatar Jun 20 '24 14:06 openjdk[bot]

/sponsor

lahodaj avatar Jun 21 '24 10:06 lahodaj

Going to push as commit 08ace27da1d9cd215c77471eabf41417ff6282d2. Since your change was applied there have been 63 commits pushed to the master branch:

  • 711e7238196a4ef9211ed4cca15c7c1d774df019: 6967482: TAB-key does not work in JTables after selecting details-view in JFileChooser
  • d2bebffb1fd26fae4526afd33a818ee776b7102e: 8327370: (ch) sun.nio.ch.Poller.register throws AssertionError
  • ed149062d0e8407710f083aa85d28d27c4a45ecc: 8333361: ubsan,test : libHeapMonitorTest.cpp:518:9: runtime error: null pointer passed as argument 2, which is declared to never be null
  • bdd96604ae55ba0cd3cd3363e2ba44205d8aa3aa: 8323196: jdk/jfr/api/consumer/filestream/TestOrdered.java failed with "Events are not ordered! Reuse = false"
  • 6a5cb0b2c49cb390ce8b87fd977ee79572df90fc: 8334567: [test] runtime/os/TestTracePageSizes move ppc handling
  • 880e458a1072589ae199cc9204dcce9eab0f4eaa: 8333819: Move embedded external addresses from relocation info into separate global table
  • e5de26ddf0550da9e6d074d5b9ab4a943170adca: 8329032: C2 compiler register allocation support for APX EGPRs
  • 4b4a483b6fe7a6fcfdfe6f68faac29099a64c982: 8330699: Obsolete -XX:+UseEmptySlotsInSupers
  • 187710e1c1714ba28c7802efd4f7bb32a366d79d: 8333300: [JVMCI] add support for generational ZGC
  • de8ee97718d7e12b541b310cf5b67f3e10e91ad9: 8334333: MissingResourceCauseTestRun.java fails if run by root
  • ... and 53 more: https://git.openjdk.org/jdk/compare/8464ce6db5cbd5d50ac2a2bcba905b7255f510f5...master

Your commit was automatically rebased without conflicts.

openjdk[bot] avatar Jun 21 '24 10:06 openjdk[bot]

@lahodaj @archiecobbs Pushed as commit 08ace27da1d9cd215c77471eabf41417ff6282d2.

:bulb: You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

openjdk[bot] avatar Jun 21 '24 10:06 openjdk[bot]