Hannes, Differences between HTML representations of "()" and "(...)" are an inconsistency. I agree that it's a separate issue and I think we should fix it.
On a related note, it's impressive to see how a trivial change like that ripples through the tests that use string comparison. Although string comparison is very flexible, it's a maintenance nightmare. This and some other situations suggest we should make test assertions using higher-level primitives. -Pavel > On 30 Jul 2020, at 08:59, Hannes Wallnoefer <[email protected]> > wrote: > > Thanks, Pavel! > >> Am 29.07.2020 um 16:42 schrieb Pavel Rappo <[email protected]>: >> >> Thanks for doing this. >> >> It seems possible for an empty list of parameters "()" to be of one style, >> while a non-empty list to be of another: AbstractMemberWriter.java:689:694 > > That empty parameters are not wrapped in a parameters span is the way it was > before, I just added the comment because `parameters.charCount() == 2` is a > bit less obvious than `parameters.isEmpty()`. > > I considered and tried changing that as part of this patch, but it more than > doubled the ten tests that needed to be updated, and it didn’t seem to be > necessary for the issue being fixed. > > However, I’m open to making that change either as part of this issue or as a > new issue. > > Hannes > > >> If so, should we fix that too? >> >> -Pavel >> >>> On 28 Jul 2020, at 13:45, Hannes Wallnoefer <[email protected]> >>> wrote: >>> >>> Please review: >>> >>> JBS: https://bugs.openjdk.java.net/browse/JDK-8241518 >>> Webrev: http://cr.openjdk.java.net/~hannesw/8241518/webrev.00/ >>> API docs: http://cr.openjdk.java.net/~hannesw/8241518/api.00/ >>> >>> This changes the „parameters“ span in member signatures to contain both >>> opening and closing parens, whereas previously it only contained the >>> closing parens. >>> >>> To preserve visual alignment of parameters, a space character is added >>> after each line break added to the parameter list. Thus, the new code >>> renders method signatures as shown below with the box representing the >>> „parameters“ span: >>> __________ >>> methodName|(int p1, | >>> | int p2, | >>> | int p3) | >>> |__________| >>> >>> Previously it was rendered the following way: >>> _________ >>> methodName(|int p1, | >>> |int p2, | >>> |int p3) | >>> |_________| >>> >>> IMO this also improves layout when method name and parameters don’t fit the >>> browser width, as the whole parameter span including both parens is now >>> broken to a new line, preserving its layout with the single space >>> indentation. >>> >>> Thanks, >>> Hannes >> >
