On Tue, 4 Jun 2024 05:30:36 GMT, David Holmes <dhol...@openjdk.org> wrote:
>> src/hotspot/share/runtime/javaThread.cpp line 1832:
>>
>>> 1830: st->print("\t");
>>> 1831: indentation--;
>>> 1832: }
>>
>> Suggestion:
>>
>> while (indentation-- > 0) {
>> st->print("\t");
>> }
>>
>> Though not sure using `\t` is the best way to indent this as stream
>> indentation is based on spaces.
>
> Actually I'm not sure what this indentation actually does at this location
> and its affect on other user's of this API. I would have expected the caller
> to set up the necessary indentation in the stream that gets passed in. I see
> you inc the indentation but then use the current indentation to insert
> multiple tabs - which should not be necessary if the stream indentation works
> correctly. ???
Note that We are in the process of adding better and saner auto-indentation to
outputStream. See https://github.com/openjdk/jdk/pull/19461 . I don't think
that PR is going to take long.
If you don't want to wait, please:
- As David wrote, use spaces, not tabs
- Today's pattern for using outputStream indentation is:
- set up indentation, preferably with streamIndentor, not manually with
inc/dec
- then, before printing each line, call stream->indent()
This pattern would also help us to later identify and remove this manual
indentation pattern if auto-indent becomes a thing.
But really, waiting for https://github.com/openjdk/jdk/pull/19461 would be
preferable. Then, all you have to do is place a streamIndentor around stack
printing. Sub-function printing is then indented automatically.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/19482#discussion_r1625622263
