On Tue, 31 Aug 2021 00:36:51 GMT, Jakob Cornell <github.com+5642931+jakobcorn...@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. > > Jakob Cornell has refreshed the contents of this pull request, and previous > commits have been removed. The incremental views will show differences > compared to the previous content of the PR. I think the main reason the diff was so unhelpful is that part of my original change was converting the `<n> <command>` handling from recursive to iterative. I think the code as a whole is more intuitive when this is done iteratively, and along with the iterative approach came a fix (for synchronous commands) for a minor bug in which the prompt is printed along with each iteration of an `<n> <command>` command. However, reverting to the recursive approach and restoring some original variable names has made the diff much easier to understand. I do have the iterative version saved on a local branch in case it becomes useful. I've run the tests you indicated (`make run-test-vmTestbase_nsk_jdb`) and those are all passing. I'll take a look into writing some new tests for the changes. I think the main reason the diff was so unhelpful is that part of my original change was converting the `<n> <command>` handling from recursive to iterative. I think the code as a whole is more intuitive when this is done iteratively, and along with the iterative approach came a fix (for synchronous commands) for a minor bug in which the prompt is printed along with each iteration of an `<n> <command>` command. However, reverting to the recursive approach and restoring some original variable names has made the diff much easier to understand. I do have the iterative version saved on a local branch in case it becomes useful. I've run the tests you indicated (`make run-test-vmTestbase_nsk_jdb`) and those are all passing. I'll take a look into writing some new tests for the changes. ------------- PR: https://git.openjdk.java.net/jdk/pull/5290
