Hi Kumar,
Thanks for spotting this. The cause is the equivalent strange indent in
the original source. The conversion program does not change the
preceding indent for the opening `"""`. The body of the text block then
follows the indent of the `"""`.
The main change has already been pushed, and it was deliberate that we
did not mix manual and automated changes. This instance be fixed up in a
followup changeset.
-- Jon
On 5/8/20 7:08 AM, Kumar Srinivasan wrote:
Hi Jon,
I generally like this…:)
I noticed a strange indent at line #70
http://cr.openjdk.java.net/~jjg/8242532/webrev.01/test/langtools/jdk/javadoc/doclet/constantValues/TestConstantValuesDriver.java.frames.html
I did not check every file and diff it is too laborious.
Thanks
Kumar
On May 5, 2020, at 9:35 AM, Jonathan Gibbons
<jonathan.gibb...@oracle.com <mailto:jonathan.gibb...@oracle.com>> wrote:
Thanks for the +1, but I will do one more pass to tweak the
parameters for very long lines and for short lines.
-- Jon
On 5/5/20 9:12 AM, Hannes Wallnoefer wrote:
Yes, I noted that text blocks are not that great with string
concatenation.
I agree with all you say, as I said, these are all questions of
taste and it’s easy to get lost in them. I also think you spent a
good amount of time tweaking the parameters already, and the result
looks very good.
So +1 for the patch as it is.
Hannes
Am 05.05.2020 um 18:07 schrieb Jonathan Gibbons
<jonathan.gibb...@oracle.com <mailto:jonathan.gibb...@oracle.com>>:
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 <mailto: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://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.openjdk.java.net%2Fbrowse%2FJDK-8242532&data=02%7C01%7Ckusrinivasan%40vmware.com%7Cbb8b29b6f8f9469eb2ad08d7f1132cfc%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637242936974359927&sdata=NkJlAx3BJaMTkd0Wq4WLJGGvTEDk4PcYtVEq2lNxYvI%3D&reserved=0
Webrev:https://nam04.safelinks.protection.outlook.com/?url=http:%2F%2Fcr.openjdk.java.net%2F~jjg%2F8242532%2Fwebrev.01%2F&data=02%7C01%7Ckusrinivasan%40vmware.com%7Cbb8b29b6f8f9469eb2ad08d7f1132cfc%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637242936974359927&sdata=JiVQI%2BTy1ffJfe2gOMiVXhC9x4WQd8oNMG2nOvtBY6k%3D&reserved=0
