Skip to content

Remove name parameter from animation directive. #1715

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jun 14, 2018
Merged

Remove name parameter from animation directive. #1715

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
235 changes: 160 additions & 75 deletions lib/src/model.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import 'package:analyzer/src/generated/source_io.dart';
import 'package:analyzer/src/dart/element/member.dart'
show ExecutableMember, Member, ParameterMember;
import 'package:analyzer/src/dart/analysis/driver.dart';
import 'package:args/args.dart';
import 'package:collection/collection.dart';
import 'package:dartdoc/src/dartdoc_options.dart';
import 'package:dartdoc/src/element_type.dart';
Expand Down Expand Up @@ -3487,12 +3488,9 @@ abstract class ModelElement extends Canonicalization
///
/// {@example PATH [region=NAME] [lang=NAME]}
///
/// where PATH and NAME are tokens _without_ whitespace; NAME can optionally be
/// quoted (use of quotes is for backwards compatibility and discouraged).
///
/// If PATH is `dir/file.ext` and region is `r` then we'll look for the file
/// named `dir/file-r.ext.md`, relative to the project root directory (of the
/// project for which the docs are being generated).
/// named `dir/file-r.ext.md`, relative to the project root directory of the
/// project for which the docs are being generated.
///
/// Examples: (escaped in this comment to show literal values in dartdoc's
/// dartdoc)
Expand All @@ -3505,6 +3503,10 @@ abstract class ModelElement extends Canonicalization
RegExp exampleRE = new RegExp(r'{@example\s+([^}]+)}');
return rawdocs.replaceAllMapped(exampleRE, (match) {
var args = _getExampleArgs(match[1]);
if (args == null) {
// Already warned about an invalid parameter if this happens.
return '';
}
var lang =
args['lang'] ?? pathLib.extension(args['src']).replaceFirst('.', '');

Expand Down Expand Up @@ -3533,17 +3535,19 @@ abstract class ModelElement extends Canonicalization
///
/// Syntax:
///
/// {@animation NAME WIDTH HEIGHT URL}
/// {@animation WIDTH HEIGHT URL [id=ID]}
///
/// Example:
///
/// {@animation my_video 300 300 https://example.com/path/to/video.mp4}
/// {@animation 300 300 https://example.com/path/to/video.mp4 id="my_video"}
///
/// Which will render the HTML necessary for embedding a simple click-to-play
/// HTML5 video player with no controls.
/// HTML5 video player with no controls that has an HTML id of "my_video".
///
/// The NAME should be a unique name that is a valid javascript identifier,
/// and will be used as the id for the video tag.
/// The optional ID should be a unique id that is a valid JavaScript
/// identifier, and will be used as the id for the video tag. If no ID is
/// supplied, then a unique identifier (starting with "animation_") will be
/// generated.
///
/// The width and height must be integers specifying the dimensions of the
/// video file in pixels.
Expand All @@ -3554,87 +3558,114 @@ abstract class ModelElement extends Canonicalization
final RegExp basicAnimationRegExp =
new RegExp(r'''{@animation\s+([^}]+)}''');

// Animations have four parameters, and the last one can be surrounded by
// quotes (which are ignored). This RegExp is used to validate the directive
// for the correct number of parameters.
final RegExp animationRegExp =
new RegExp(r'''{@animation\s+([^}\s]+)\s+([^}\s]+)\s+([^}\s]+)'''
r'''\s+['"]?([^}]+)['"]?}''');

// Matches valid javascript identifiers.
final RegExp validNameRegExp = new RegExp(r'^[a-zA-Z_][a-zA-Z0-9_]*$');

// Keeps names unique.
final Set<String> uniqueNames = new Set<String>();
final RegExp validIdRegExp = new RegExp(r'^[a-zA-Z_]\w*$');

final Set<String> uniqueIds = new Set<String>();
String getUniqueId(String base) {
int count = 1;
String id = '$base$count';
while (uniqueIds.contains(id)) {
count++;
id = '$base$count';
}
return id;
}

return rawDocs.replaceAllMapped(basicAnimationRegExp, (basicMatch) {
final Match match = animationRegExp.firstMatch(basicMatch[0]);
if (match == null) {
final ArgParser parser = new ArgParser();
parser.addOption('id');
final ArgResults args = _parseArgs(basicMatch[1], parser, 'animation');
if (args == null) {
// Already warned about an invalid parameter if this happens.
return '';
}
final List<String> positionalArgs = args.rest.sublist(0);
String uniqueId;
bool wasDeprecated = false;
if (positionalArgs.length == 4) {
// Supports the original form of the animation tag for backward
// compatibility.
uniqueId = positionalArgs.removeAt(0);
Copy link
Contributor

Choose a reason for hiding this comment

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

This will of course completely discard the name. Given that anchors do not actually work yet I suppose this is probably harmless?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, this is just is converting the old form of positional args (four args) to the new form (three args) by removing the first arg and using it as the uniqueName. The id is actually used in the injected HTML to control the playing of the video, so the uniqueness is required, as is the presence of some sort of id.

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, got it.

wasDeprecated = true;
} else if (positionalArgs.length == 3) {
uniqueId = args['id'] ?? getUniqueId('animation_');
} else {
warn(PackageWarning.invalidParameter,
message: 'Invalid @animation directive: ${basicMatch[0]}\n'
'Animation directives must be of the form: {@animation NAME '
'WIDTH HEIGHT URL}');
message: 'Invalid @animation directive, "${basicMatch[0]}"\n'
'Animation directives must be of the form "{@animation WIDTH '
'HEIGHT URL [id=ID]}"');
return '';
}
String name = match[1];
if (!validNameRegExp.hasMatch(name)) {

if (!validIdRegExp.hasMatch(uniqueId)) {
warn(PackageWarning.invalidParameter,
message: 'An animation has an invalid name: $name. The name can '
'only contain letters, numbers and underscores.');
message: 'An animation has an invalid identifier, "$uniqueId". The '
'identifier can only contain letters, numbers and underscores, '
'and must not begin with a number.');
return '';
}
if (uniqueIds.contains(uniqueId)) {
warn(PackageWarning.invalidParameter,
message: 'An animation has a non-unique identifier, "$uniqueId". '
'Animation identifiers must be unique.');
return '';
} else {
if (uniqueNames.contains(name)) {
warn(PackageWarning.invalidParameter,
message:
'An animation has a non-unique name: $name. Animation names '
'must be unique.');
return '';
}
uniqueNames.add(name);
}
uniqueIds.add(uniqueId);

int width;
try {
width = int.parse(match[2]);
width = int.parse(positionalArgs[0]);
} on FormatException {
warn(PackageWarning.invalidParameter,
message:
'An animation has an invalid width ($name): ${match[2]}. The '
'width must be an integer.');
message: 'An animation has an invalid width ($uniqueId), '
'"${positionalArgs[0]}". The width must be an integer.');
return '';
}

int height;
try {
height = int.parse(match[3]);
height = int.parse(positionalArgs[1]);
} on FormatException {
warn(PackageWarning.invalidParameter,
message:
'An animation has an invalid height ($name): ${match[3]}. The '
'height must be an integer.');
message: 'An animation has an invalid height ($uniqueId), '
'"${positionalArgs[1]}". The height must be an integer.');
return '';
}

Uri movieUrl;
try {
movieUrl = Uri.parse(match[4]);
movieUrl = Uri.parse(positionalArgs[2]);
} on FormatException catch (e) {
warn(PackageWarning.invalidParameter,
message:
'An animation URL could not be parsed ($name): ${match[4]}\n$e');
message: 'An animation URL could not be parsed ($uniqueId): '
'${positionalArgs[2]}\n$e');
return '';
}
final String overlayName = '${name}_play_button_';
final String overlayId = '${uniqueId}_play_button_';

// Only warn about deprecation if some other warning didn't occur.
if (wasDeprecated) {
warn(PackageWarning.deprecated,
message:
'Deprecated form of @animation directive, "${basicMatch[0]}"\n'
'Animation directives are now of the form "{@animation '
'WIDTH HEIGHT URL [id=ID]}" (id is an optional '
'parameter)');
}

// Blank lines before and after, and no indenting at the beginning and end
// is needed so that Markdown doesn't confuse this with code, so be
// careful of whitespace here.
return '''

<div style="position: relative;">
<div id="${overlayName}"
onclick="if ($name.paused) {
$name.play();
<div id="${overlayId}"
onclick="if ($uniqueId.paused) {
$uniqueId.play();
this.style.display = 'none';
} else {
$name.pause();
$uniqueId.pause();
this.style.display = 'block';
}"
style="position:absolute;
Expand All @@ -3645,14 +3676,14 @@ abstract class ModelElement extends Canonicalization
background-repeat: no-repeat;
background-image: url(static-assets/play_button.svg);">
</div>
<video id="$name"
<video id="$uniqueId"
style="width:${width}px; height:${height}px;"
onclick="if (this.paused) {
this.play();
$overlayName.style.display = 'none';
$overlayId.style.display = 'none';
} else {
this.pause();
$overlayName.style.display = 'block';
$overlayId.style.display = 'block';
}" loop>
<source src="$movieUrl" type="video/mp4"/>
</video>
Expand Down Expand Up @@ -3718,29 +3749,80 @@ abstract class ModelElement extends Canonicalization
});
}

/// Helper to process arguments given as a (possibly quoted) string.
///
/// First, this will split the given [argsAsString] into separate arguments,
/// taking any quoting (either ' or " are accepted) into account, including
/// handling backslash-escaped quotes.
///
/// Then, it will prepend "--" to any args that start with an identifier
/// followed by an equals sign, allowing the argument parser to treat any
/// "foo=bar" argument as "--foo=bar". It does handle quoted args like
/// "foo='bar baz'" too, returning just bar (without quotes) for the foo
/// value.
///
/// It then parses the resulting argument list normally with [argParser] and
/// returns the result.
ArgResults _parseArgs(
String argsAsString, ArgParser argParser, String directiveName) {
// Regexp to take care of splitting arguments, and handling the quotes
// around arguments, if any.
//
// Match group 1 is the "foo=" part of the option, if any.
// Match group 2 contains the quote character used (which is discarded).
// Match group 3 is a quoted arg, if any, without the quotes.
// Match group 4 is the unquoted arg, if any.
final RegExp argMatcher = new RegExp(r'([a-zA-Z\-_0-9]+=)?' // option name
r'(?:' // Start a new non-capture group for the two possibilities.
r'''(["'])((?:\\{2})*|(?:.*?[^\\](?:\\{2})*))\2|''' // with quotes.
r'([^ ]+))'); // without quotes.
final Iterable<Match> matches = argMatcher.allMatches(argsAsString);

// Remove quotes around args, and for any args that look like assignments
// (start with valid option names followed by an equals sign), add a "--" in front
// so that they parse as options.
final Iterable<String> args = matches.map<String>((Match match) {
String option = '';
if (match[1] != null) {
option = '--${match[1]}';
}
return option + (match[3] ?? '') + (match[4] ?? '');
});

try {
return argParser.parse(args);
} on ArgParserException catch (e) {
warn(PackageWarning.invalidParameter,
message: 'The {@$directiveName ...} directive was called with '
'invalid parameters. $e');
return null;
}
}

/// Helper for _injectExamples used to process @example arguments.
/// Returns a map of arguments. The first unnamed argument will have key 'src'.
/// The computed file path, constructed from 'src' and 'region' will have key
/// 'file'.
Map<String, String> _getExampleArgs(String argsAsString) {
// Extract PATH and return is under key 'src'
var endOfSrc = argsAsString.indexOf(' ');
if (endOfSrc < 0) endOfSrc = argsAsString.length;
var src = argsAsString.substring(0, endOfSrc);
src = src.replaceAll('/', Platform.pathSeparator);
final args = {'src': src};

// Process remaining named arguments
var namedArgs = argsAsString.substring(endOfSrc);
// Arg value: allow optional quotes; warning: we still don't support whitespace.
RegExp keyValueRE = new RegExp('(\\w+)=[\'"]?(\\S*)[\'"]?');
Iterable<Match> matches = keyValueRE.allMatches(namedArgs);
matches.forEach((match) {
// Ignore optional quotes
args[match[1]] = match[2].replaceAll(new RegExp('[\'"]'), '');
});
ArgParser parser = new ArgParser();
parser.addOption('lang');
parser.addOption('region');
ArgResults results = _parseArgs(argsAsString, parser, 'example');
if (results == null) {
return null;
}

// Compute 'file'
// Extract PATH and fix the path separators.
final String src = results.rest.isEmpty
? ''
: results.rest.first.replaceAll('/', Platform.pathSeparator);
final Map<String, String> args = <String, String>{
'src': src,
'lang': results['lang'],
'region': results['region'] ?? '',
};

// Compute 'file' from region and src.
final fragExtension = '.md';
var file = src + fragExtension;
var region = args['region'] ?? '';
Expand Down Expand Up @@ -4233,6 +4315,9 @@ class PackageGraph {
case PackageWarning.invalidParameter:
warningMessage = 'invalid parameter to dartdoc directive: ${message}';
break;
case PackageWarning.deprecated:
warningMessage = 'deprecated dartdoc usage: ${message}';
break;
}

List<String> messageParts = [warningMessage];
Expand Down
5 changes: 5 additions & 0 deletions lib/src/warnings.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,10 @@ final Map<PackageWarning, PackageWarningHelpText> packageWarningText = const {
PackageWarning.invalidParameter,
"invalidParameter",
"A parameter given to a dartdoc directive was invalid."),
PackageWarning.deprecated: const PackageWarningHelpText(
PackageWarning.deprecated,
"deprecated",
"A dartdoc directive has a deprecated format."),
};

/// Something that package warnings can be called on. Optionally associated
Expand Down Expand Up @@ -130,6 +134,7 @@ enum PackageWarning {
missingFromSearchIndex,
typeAsHtml,
invalidParameter,
deprecated,
}

/// Warnings it is OK to skip if we can determine the warnable isn't documented.
Expand Down
Loading