RFR: 8271356: Modify jdb to treat an empty command as a repeat of the previous command

[Chris Plummer]

On Sat, 28 Aug 2021 05:48:47 GMT, Jakob Cornell <github.com+5642931+jakobcornell at openjdk.org> wrote:

> This has been under discussion on and off for the past month or so on serviceability-dev, and I think a CSR request is required, so this may be a work in progress.
> 
> Notes on the patch:
> 
> - The `list` command previously marked a line in each listing with `=>`.  In a bare `list` this is the next line up for execution.  Previously when requesting a specific location (e.g. `list 5`) the requested line would be marked.  With the patch applied, `list` will only ever mark the next line up for execution.  This is consistent with the behavior of GDB and PDB (at least).
> - `EOF` is printed when the repeat setting is on and a bare `list` command follows a listing containing the last source line.  This feature is from PDB; it's a somewhat softer message than the one for an explicit `list` request that's out of range.
> - I don't speak Chinese or Japanese, so I've omitted localizations for the new messages in those locales.  However, I updated the help text in both to include the new commands, with the descriptions left empty for now.

I'm not sure how to advise about the Chinese or Japanese localization. I don't know what the process is for getting the text updated and properly tested. Hopefully someone else has that knowledge and will comment.

What testing have you done? There are jdb tests in `test/hotspot/jtreg/vmTestbase/nsk/jdb`. You'll also need to write some tests for the new functionality.

src/jdk.jdi/share/classes/com/sun/tools/example/debug/tty/TTY.java line 441:

> 439:      */
> 440:     String executeCommand(StringTokenizer t) {
> 441:         String name = t.nextToken().toLowerCase();

Because of this rename of "cmd" to "name", the diff below is very hard to read. It basically just shows that you removed a large section of code and replaced it with another large section of code, which isn't really the case. I think the indentation change with the introduce of `if (valid)` might also be partly to blame. it would be much easier to review if you could clean up the diff.

src/jdk.jdi/share/classes/com/sun/tools/example/debug/tty/TTY.java line 457:

> 455:             while (Character.isDigit(name.charAt(0)) && t.hasMoreTokens()) {
> 456:                 try {
> 457:                     reps *= Integer.parseInt(name);  // nested repeats are possible

I don't fully understand the relevance of the changes here to the goal of adding the empty line repeat support. Is this new functionality (nested repeats)?

src/jdk.jdi/share/classes/com/sun/tools/example/debug/tty/TTY.java line 483:

> 481:                     } else {
> 482:                         Commands evaluator = new Commands();
> 483:                         var args = new StringTokenizer(argsLine);

I think the introduction of `args` here and referenced below instead of `t` is also contributing to the issue with the diff. Can you explain why `t` can just continue to be used below?

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

PR: https://git.openjdk.java.net/jdk/pull/5290