Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Format parameters better #3731

Merged
merged 2 commits into from
Mar 26, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion lib/resources/styles.css
Original file line number Diff line number Diff line change
@@ -504,6 +504,21 @@ dl dt.callable .name {
padding-inline-start: unset;
}

.parameter-list.single-line {
display: inline;
margin-left: 0;
}

.parameter-list.single-line > li {
display: inline;
}

.parameter-list.single-line > li > .parameter {
display: inline;
margin-left: 0;
text-indent: 0;
}

.signature {
color: var(--main-text-color);
}
@@ -726,8 +741,9 @@ td {
}

.multi-line-signature .parameter {
margin-left: 24px;
margin-left: 60px;
display: block;
text-indent: -36px;
}

.breadcrumbs {
9 changes: 9 additions & 0 deletions lib/src/model/model_element.dart
Original file line number Diff line number Diff line change
@@ -690,11 +690,20 @@ abstract class ModelElement extends Canonicalization
ParameterRenderer get _parameterRendererDetailed =>
const ParameterRendererHtmlList();

/// The list of linked parameters, as inline HTML, including metadata.
///
/// The text does not contain the leading or trailing parentheses.
String get linkedParams => _parameterRenderer.renderLinkedParams(parameters);

/// The list of linked parameters, as block HTML, including metadata.
///
/// The text does not contain the leading or trailing parentheses.
String get linkedParamsLines =>
_parameterRendererDetailed.renderLinkedParams(parameters).trim();

/// The list of linked parameters, as inline HTML, without metadata.
///
/// The text does not contain the leading or trailing parentheses.
String? get linkedParamsNoMetadata =>
_parameterRenderer.renderLinkedParams(parameters, showMetadata: false);

168 changes: 107 additions & 61 deletions lib/src/render/parameter_renderer.dart
Original file line number Diff line number Diff line change
@@ -11,26 +11,55 @@ import 'package:dartdoc/src/model/parameter.dart';
class ParameterRendererHtmlList extends ParameterRendererHtml {
const ParameterRendererHtmlList();

@override
bool get isBlock => true;

@override
String listItem(String item) => '<li>$item</li>\n';

@override
// TODO(jcollins-g): consider comma separated lists and more advanced CSS.
String orderedList(String listItems) =>
'<ol class="parameter-list">$listItems</ol>\n';
String orderedList(
String listItems, {
bool multiLine = false,
String openingBracket = '',
String closingBracket = '',
}) {
var classes = [
'parameter-list',
if (!multiLine) 'single-line',
];
return '$openingBracket'
'<ol class="${classes.join(' ')}"> $listItems</ol>'
'$closingBracket';
}
}

/// Render HTML suitable for a single, wrapped line.
class ParameterRendererHtml extends ParameterRenderer {
const ParameterRendererHtml();

@override
bool get isBlock => false;

@override
String listItem(String item) => item;

@override
String orderedList(String listItems) => listItems;
String orderedList(
String listItems, {
bool multiLine = false,
String openingBracket = '',
String closingBracket = '',
}) =>
'$openingBracket$listItems$closingBracket';

@override
String annotation(String annotation) => '<span>$annotation</span>';

@override
String covariant(String covariant) => '<span>$covariant</span>';

@override
String defaultValue(String defaultValue) {
var escaped =
@@ -41,21 +70,31 @@ class ParameterRendererHtml extends ParameterRenderer {
@override
String parameter(String parameter, String id) =>
'<span class="parameter" id="$id">$parameter</span>';

@override
String parameterName(String parameterName) =>
'<span class="parameter-name">$parameterName</span>';

@override
String typeName(String typeName) =>
'<span class="type-annotation">$typeName</span>';

@override
String required(String required) => '<span>$required</span>';
}

abstract class ParameterRenderer {
const ParameterRenderer();

bool get isBlock;

String listItem(String item);
String orderedList(String listItems);
String orderedList(
String listItems, {
bool multiLine = false,
String openingBracket = '',
String closingBracket = '',
});
String annotation(String annotation);
String covariant(String covariant);
String defaultValue(String defaultValue);
@@ -66,6 +105,10 @@ abstract class ParameterRenderer {

String renderLinkedParams(List<Parameter> parameters,
{bool showMetadata = true}) {
if (parameters.isEmpty) {
return '';
}

var positionalParams = parameters
.where((Parameter p) => p.isRequiredPositional)
.toList(growable: false);
@@ -74,77 +117,79 @@ abstract class ParameterRenderer {
.toList(growable: false);
var namedParams =
parameters.where((Parameter p) => p.isNamed).toList(growable: false);
var isMultiline =
isBlock && (namedParams.isNotEmpty || parameters.length > 3);

var buffer = StringBuffer();
if (positionalParams.isNotEmpty) {
_renderLinkedParameterSublist(
positionalParams,
buffer,
trailingComma:
optionalPositionalParams.isNotEmpty || namedParams.isNotEmpty,
showMetadata: showMetadata,
);

// If there are no required parameters, but optional positional or named
// parameters, then we start with an opening bracket.
var openingBracket = '';
// If there are any optional positional or named parameters, then we end
// with a closing bracket.
var closingBracket = '';

if (positionalParams.isEmpty) {
if (optionalPositionalParams.isNotEmpty) {
openingBracket = '[';
} else if (namedParams.isNotEmpty) {
openingBracket = '{';
}
} else {
// Determine whether the line should end with an opening delimiter for
// optional parameters.
String trailingText;
if (optionalPositionalParams.isNotEmpty) {
trailingText = ', [';
} else if (namedParams.isNotEmpty) {
trailingText = ', {';
} else if (isMultiline) {
trailingText = ', ';
} else {
trailingText = '';
}

for (var p in positionalParams) {
var suffix = identical(p, positionalParams.last) ? trailingText : ', ';
buffer.write(
_renderParameter(p, showMetadata: showMetadata, suffix: suffix));
}
}
if (optionalPositionalParams.isNotEmpty) {
_renderLinkedParameterSublist(
optionalPositionalParams,
buffer,
trailingComma: namedParams.isNotEmpty,
openBracket: '[',
closeBracket: ']',
showMetadata: showMetadata,
);
closingBracket = ']';

for (var p in optionalPositionalParams) {
var suffix = isMultiline || !identical(p, optionalPositionalParams.last)
? ', '
: '';
buffer.write(
_renderParameter(p, showMetadata: showMetadata, suffix: suffix));
}
}
if (namedParams.isNotEmpty) {
_renderLinkedParameterSublist(
namedParams,
buffer,
trailingComma: false,
openBracket: '{',
closeBracket: '}',
showMetadata: showMetadata,
);
}
return orderedList(buffer.toString());
}
closingBracket = '}';

void _renderLinkedParameterSublist(
List<Parameter> parameters,
StringBuffer buffer, {
required bool trailingComma,
String openBracket = '',
String closeBracket = '',
bool showMetadata = true,
}) {
for (var p in parameters) {
var prefix = '';
var suffix = '';
if (identical(p, parameters.first)) {
prefix = openBracket;
}
if (identical(p, parameters.last)) {
suffix += closeBracket;
if (trailingComma) suffix += ', ';
} else {
suffix += ', ';
for (var p in namedParams) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make a helper to avoid the duplicate logic of each parameter type?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I was able to fold more things into _renderParameter, much simpler. Thanks!

var suffix = isMultiline || !identical(p, namedParams.last) ? ', ' : '';
buffer.write(
_renderParameter(p, showMetadata: showMetadata, suffix: suffix));
}
final renderedParameter = _renderParameter(
p,
prefix: prefix,
suffix: suffix,
showMetadata: showMetadata,
);
buffer.write(listItem(parameter(renderedParameter, p.htmlId)));
}

return orderedList(
buffer.toString(),
multiLine: isMultiline,
openingBracket: openingBracket,
closingBracket: closingBracket,
);
}

String _renderParameter(
Parameter param, {
required String prefix,
required bool showMetadata,
required String suffix,
bool showMetadata = true,
}) {
final buffer = StringBuffer(prefix);
final buffer = StringBuffer();
final modelType = param.modelType;

if (showMetadata && param.hasAnnotations) {
@@ -217,6 +262,7 @@ abstract class ParameterRenderer {
}

buffer.write(suffix);
return buffer.toString();
var wrappedParameter = parameter(buffer.toString(), param.htmlId);
return listItem(wrappedParameter);
}
}
34 changes: 17 additions & 17 deletions test/end2end/model_test.dart
Original file line number Diff line number Diff line change
@@ -345,14 +345,14 @@ void main() async {
expect(
m1.linkedParamsLines,
equals(
'<ol class="parameter-list"><li><span class="parameter" id="m1-param-some"><span class="type-annotation">int</span> <span class="parameter-name">some</span>, </span></li>\n'
'<ol class="parameter-list"> <li><span class="parameter" id="m1-param-some"><span class="type-annotation">int</span> <span class="parameter-name">some</span>, </span></li>\n'
'<li><span class="parameter" id="m1-param-regular"><span class="type-annotation">dynamic</span> <span class="parameter-name">regular</span>, </span></li>\n'
'<li><span class="parameter" id="m1-param-parameters"><span>covariant</span> <span class="type-annotation">dynamic</span> <span class="parameter-name">parameters</span>, </span></li>\n'
'<li><span class="parameter" id="m1-param-p1">{<span>required</span> <span class="type-annotation">dynamic</span> <span class="parameter-name">p1</span>, </span></li>\n'
'<li><span class="parameter" id="m1-param-parameters"><span>covariant</span> <span class="type-annotation">dynamic</span> <span class="parameter-name">parameters</span>, {</span></li>\n'
'<li><span class="parameter" id="m1-param-p1"><span>required</span> <span class="type-annotation">dynamic</span> <span class="parameter-name">p1</span>, </span></li>\n'
'<li><span class="parameter" id="m1-param-p2"><span class="type-annotation">int</span> <span class="parameter-name">p2</span> = <span class="default-value">3</span>, </span></li>\n'
'<li><span class="parameter" id="m1-param-p3"><span>required</span> <span>covariant</span> <span class="type-annotation">dynamic</span> <span class="parameter-name">p3</span>, </span></li>\n'
'<li><span class="parameter" id="m1-param-p4"><span>required</span> <span>covariant</span> <span class="type-annotation">int</span> <span class="parameter-name">p4</span>}</span></li>\n'
'</ol>'));
'<li><span class="parameter" id="m1-param-p4"><span>required</span> <span>covariant</span> <span class="type-annotation">int</span> <span class="parameter-name">p4</span>, </span></li>\n'
'</ol>}'));
});

test('verify no regression on ordinary optionals', () {
@@ -369,11 +369,11 @@ void main() async {
expect(
m2.linkedParamsLines,
equals(
'<ol class="parameter-list"><li><span class="parameter" id="m2-param-sometimes"><span class="type-annotation">int</span> <span class="parameter-name">sometimes</span>, </span></li>\n'
'<li><span class="parameter" id="m2-param-we"><span class="type-annotation">dynamic</span> <span class="parameter-name">we</span>, </span></li>\n'
'<li><span class="parameter" id="m2-param-have">[<span class="type-annotation">String</span> <span class="parameter-name">have</span>, </span></li>\n'
'<li><span class="parameter" id="m2-param-optionals"><span class="type-annotation">double</span> <span class="parameter-name">optionals</span>]</span></li>\n'
'</ol>'));
'<ol class="parameter-list"> <li><span class="parameter" id="m2-param-sometimes"><span class="type-annotation">int</span> <span class="parameter-name">sometimes</span>, </span></li>\n'
'<li><span class="parameter" id="m2-param-we"><span class="type-annotation">dynamic</span> <span class="parameter-name">we</span>, [</span></li>\n'
'<li><span class="parameter" id="m2-param-have"><span class="type-annotation">String</span> <span class="parameter-name">have</span>, </span></li>\n'
'<li><span class="parameter" id="m2-param-optionals"><span class="type-annotation">double</span> <span class="parameter-name">optionals</span>, </span></li>\n'
'</ol>]'));
});

test('anonymous callback parameters are correctly marked as nullable', () {
@@ -383,8 +383,8 @@ void main() async {
expect(listen.isRequiredPositional, isTrue);
expect(onDone.isNamed, isTrue);

expect(m3.linkedParamsLines, contains('</ol>\n)?, '));
expect(m3.linkedParamsLines, contains('</ol>\n)?}</span>'));
expect(m3.linkedParamsLines, contains('</ol>)?, '));
expect(m3.linkedParamsLines, contains('</ol>}'));
});

test('Late final class member test', () {
@@ -3245,7 +3245,7 @@ void main() async {
topLevelFunction2.linkedParamsLines,
contains('<span class="parameter-name">p3</span> = '
'<span class="default-value">const &lt;String, int&gt;{}</span>'
']</span>'));
'</span>'));
});

test('has source code', () {
@@ -3288,11 +3288,11 @@ String? topLevelFunction(int param1, bool param2, Cool coolBeans,
.renderLinkedParams(doAComplicatedThing.parameters);
expect(
params,
'<span class="parameter" id="doAComplicatedThing-param-x"><span class="type-annotation">int</span> <span class="parameter-name">x</span>, </span>'
'<span class="parameter" id="doAComplicatedThing-param-doSomething">{<span class="type-annotation">void</span> <span class="parameter-name">doSomething</span>(<span class="parameter" id="doSomething-param-aThingParameter"><span class="type-annotation">int</span> <span class="parameter-name">aThingParameter</span>, </span>'
'<span class="parameter" id="doAComplicatedThing-param-x"><span class="type-annotation">int</span> <span class="parameter-name">x</span>, {</span>'
'<span class="parameter" id="doAComplicatedThing-param-doSomething"><span class="type-annotation">void</span> <span class="parameter-name">doSomething</span>(<span class="parameter" id="doSomething-param-aThingParameter"><span class="type-annotation">int</span> <span class="parameter-name">aThingParameter</span>, </span>'
'<span class="parameter" id="doSomething-param-anotherThing"><span class="type-annotation">String</span> <span class="parameter-name">anotherThing</span></span>)?, </span>'
'<span class="parameter" id="doAComplicatedThing-param-doSomethingElse"><span class="type-annotation">void</span> <span class="parameter-name">doSomethingElse</span>(<span class="parameter" id="doSomethingElse-param-aThingParameter"><span class="type-annotation">int</span> <span class="parameter-name">aThingParameter</span>, </span>'
'<span class="parameter" id="doSomethingElse-param-somethingElse"><span class="type-annotation">double</span> <span class="parameter-name">somethingElse</span></span>)?}</span>');
'<span class="parameter" id="doSomethingElse-param-somethingElse"><span class="type-annotation">double</span> <span class="parameter-name">somethingElse</span></span>)?</span>}');
});
});

@@ -4532,7 +4532,7 @@ String? topLevelFunction(int param1, bool param2, Cool coolBeans,
);
expect(
aComplexTypedef.linkedParamsLines,
'<ol class="parameter-list">'
'<ol class="parameter-list single-line"> '
'<li><span class="parameter" id="param-"><span class="type-annotation">A3</span>, </span></li>\n'
'<li><span class="parameter" id="param-"><span class="type-annotation">String</span></span></li>\n'
'</ol>',
Loading