Hannes,
Thanks for the feedback.
It is easy enough to change the threshhold for wrapping very long
lines. I'll look at the other cases you mention, but the more we get
into style issues like "convert all or convert none", the harder it will
be to come up with rules for the converter. At some point we'll have to
declare victory, push the automated conversion, and then fix up other
cases manually, either in a single separate pass, or individually when
we next edit the respective files.
The converter program has a length threshhold; in the first iteration,
there were cases where short strings were being converted and looked
more clumsy than the original. This was particularly true in some cases
with interposed values, like
"start of message " + value + " rest of message".
-- Jon
On 5/5/20 8:55 AM, Hannes Wallnoefer wrote:
Hi Jon,
I find that using text blocks increase readability greatly, and the patch looks
mostly very good to me.
There are a few cases where I am not quite sure about the criteria used to
decide whether or not to use text blocks. Nothing too bad but I think these are
worth discussing:
TestConstructors.java line 77:
These are all short strings with a few escapes, it seems strange that one is
converted to a text block just because it is a few characters longer. I think
these strings should all be converted, or none of them. But I must admit it’s
tough to decide where to draw the line.
TestClassCrossReferences.java line starting at line 57:
I like the wrapping of very long lines into actual „blocks“, but I think the
threshold of 240 characters is too large in relation to the 80 characters
pieces we chop them into. A line of 200 characters looks a bit like it was
forgotten next to a wrapped block. IMO 120 or 160 characters would be a better
threshold, i.e. a bit longer than the ideal length but still ok.
I think my point is: chopped lines are quite easy to recognise as one big line,
so no need to avoid them so long.
TestHtmlVersion.java line 566:
TestIndexInPackageFiles.java line 60:
Why are these strings not converted? They are rather short, but they consist of
3 lines each and I think they woud benefit from text block representation.
TestSearch.java line 326:
These strings of Unicode escapes don’t seem to benefit from text block
conversion, except for the \n at the end. Is that the reason they were
included, or was it because of the unicode escapes?
Hannes
Am 01.05.2020 um 23:13 schrieb Jonathan Gibbons <jonathan.gibb...@oracle.com>:
Please review a big, but conceptually simple, change to convert JavaDoc tests
to use text blocks where reasonable.
This is an "automated" change, albeit using a custom new utility program
written for this specific purpose. There are no manual edits to any test files. There
should be no functional change in any test file.
The conversion program scans source files looking for candidates to convert into
text blocks. Candidates are long strings containing '\n' or '"' characters.
Some tests contain very long strings, that previously were manually wrapped at arbitrary
points in the string. The converter program wraps these very long strings (> 240
characters) every 80 characters. The fixed length of each "chunk" and trailing '\'
helps provide a visual distinctive block of text for the long line. Note, this algorithm
assumes there is no risk of breaking low-level Unicode sequences within the strings in the
test files.
-- Jon
JBS: https://bugs.openjdk.java.net/browse/JDK-8242532
Webrev: http://cr.openjdk.java.net/~jjg/8242532/webrev.01/
