Generate categorized pub.dev docs libraries#61
Generate categorized pub.dev docs libraries#61JSUYA wants to merge 1 commit intoflutter-tizen:mainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a new tool for generating categorized pub.dev documentation. The implementation is quite extensive, with a large Dart script handling most of the logic. The script correctly handles file operations and has good error handling with cleanup routines. I've found a few areas for improvement regarding robustness and maintainability. Specifically, the parameter parsing logic has a potential bug with unbalanced brackets, and the argument parser could be improved by using package:args. Also, a regular expression for checking command output is a bit too specific and might fail with future SDK updates. For better long-term maintainability, consider splitting the large prepare_pubdev_module_docs.dart file into smaller, more focused libraries for parsing, code generation, and file management.
| } else if (char == '>') { | ||
| if (angleDepth > 0) { | ||
| angleDepth--; | ||
| } | ||
| } else if (char == '(') { |
There was a problem hiding this comment.
The check if (angleDepth > 0) is incorrect and can lead to misparsing of parameters with unbalanced angle brackets. The depth counter should be allowed to become negative to correctly identify top-level commas for splitting parameters. The same issue exists for parenthesis and square bracket depth checking in this function.
} else if (char == '>') {
angleDepth--;
} else if (char == '(') {| _Config _parseArgs(List<String> args) { | ||
| var command = 'verify'; | ||
| var version = '6.0'; | ||
| var keepGenerated = false; | ||
|
|
||
| for (var index = 0; index < args.length; index++) { | ||
| final arg = args[index]; | ||
| switch (arg) { | ||
| case 'prepare': | ||
| case 'verify': | ||
| case 'clean': | ||
| command = arg; | ||
| case '--version': | ||
| if (index + 1 >= args.length) { | ||
| throw ArgumentError('Missing value for --version'); | ||
| } | ||
| version = args[++index]; | ||
| case '--keep-generated': | ||
| keepGenerated = true; | ||
| case '-h': | ||
| case '--help': | ||
| stdout.writeln( | ||
| 'Usage: dart run tool/prepare_pubdev_module_docs.dart ' | ||
| '[prepare|verify|clean] [--version 6.0] [--keep-generated]', | ||
| ); | ||
| exit(0); | ||
| default: | ||
| throw ArgumentError('Unknown argument: $arg'); | ||
| } | ||
| } | ||
|
|
||
| return _Config( | ||
| command: command, | ||
| version: version, | ||
| keepGenerated: keepGenerated, | ||
| ); | ||
| } |
There was a problem hiding this comment.
This manual argument parsing logic is simple but can be brittle and hard to maintain. For better maintainability and robustness, consider using the package:args library. It's the standard for parsing command-line arguments in Dart and provides features like automatic help message generation, better error reporting for invalid arguments, and a more declarative API.
| final hasOnlyWarnings = RegExp(r'Package has \d+ warning').hasMatch(output) && | ||
| !RegExp(r'Package has .*error', caseSensitive: false).hasMatch(output); |
There was a problem hiding this comment.
The regular expression to detect warnings only matches the singular "warning". It will fail if pub publish --dry-run outputs multiple warnings (e.g., "Package has 2 warnings").
final hasOnlyWarnings = RegExp(r'Package has \d+ warnings?').hasMatch(output) &&
!RegExp(r'Package has .*error', caseSensitive: false).hasMatch(output);1b09f8f to
22b7f38
Compare
This method is an example script that generates one dart file each for dartdoc_options.yaml and docs for each version.