WIP: Trying out excerpts by kevmoo · Pull Request #960 · dart-lang/core

kevmoo · 2026-05-07T20:20:12Z

No description provided.

kevmoo · 2026-05-07T20:20:55Z

1. `collection`: Non-functional Legacied `MapEquality` Constructor

The Legacy Code in README:

const MapEquality(IdentityEquality(), ListEquality());

The Bug:
This is a compile-time error! The modern MapEquality constructor expects named parameters (keys: and values:), not positional arguments. Furthermore, strict generic typing requires type arguments for the child equalities to compile under modern analysis:
```
const MapEquality<String, List<int>>(
  keys: IdentityEquality<String>(),
  values: ListEquality<int>(),
);
```
Impact: Anyone copying the legacy snippet directly from the documentation into a modern Dart codebase would be met with a compile error: Too many positional arguments: 0 expected, but 2 found.

2. `logging`: `FutureOr` Return Type Mismatch in `catchError`

The Legacy Code in README:

var future = doSomethingAsync().then((result) {
  log.fine('Got the result: $result');
  processResult(result);
}).catchError((e, stackTrace) => log.severe('Oh noes!', e, stackTrace));

The Bug:
In dart:async, catchError expects a callback that returns a FutureOr of the generic type argument. Because the doSomethingAsync().then(...) block returns void, the catch block also must return FutureOr<void> or FutureOr<Null>.
However, Logger.severe() is a void method itself, and in Dart a lambda expression => void_returning_expression evaluates to void. The analyzer raises a strict warning or error:
```
A value of type 'void' can't be returned by the 'onError' handler because it must be assignable to 'FutureOr<Null>'.
```
To make this correct, it should be written as an explicit block instead of an arrow function expression, or handle the error return types correctly:
```
var future = doSomethingAsync().then((result) {
  log.fine('Got the result: $result');
  processResult(result);
}).catchError((Object e, StackTrace stackTrace) {
  log.severe('Oh noes!', e, stackTrace);
});
```

3. `args`: Broken Command Runner Snippet Formatting

The Legacy Code in README:
In the CommitCommand.run() snippet:
```
void run() {
```
The backticks were broken and split across a code segment block midway through:
```
void run() {
```
// [argResults] is set before [run()] is called...
The Bug: This caused the generated markdown rendering to break completely in formatting on some platforms because of split nested backticks mid-method. We combined it into a single unified code block.

4. Strict Type Safety & Modern Nullability

The Legacy Code in README:
Across many README files, types in callbacks and generic arguments were entirely omitted (e.g., (e, stackTrace) in catchError).
The Bug:
Under strict type safety options, untyped error callback parameters implicitly infer to dynamic, triggering compilation warnings like inference_failure_on_untyped_parameter. Explicitly typing them as (Object e, StackTrace stackTrace) is now required for clean, warning-free compilation in new projects.

github-actions · 2026-05-07T20:25:43Z

PR Health

License Headers ✔️

// Copyright (c) 2026, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

Files
no missing headers

All source files should start with a license header.

Unrelated files missing license headers

Files
pkgs/collection/benchmark/legacy_quicksort.dart

This check can be disabled by tagging the PR with skip-license-check.

Unused Dependencies ✔️

Package	Status
args	✔️ All dependencies utilized correctly.
characters	✔️ All dependencies utilized correctly.
collection	✔️ All dependencies utilized correctly.
crypto	✔️ All dependencies utilized correctly.
logging	✔️ All dependencies utilized correctly.
os_detect	✔️ All dependencies utilized correctly.
path	✔️ All dependencies utilized correctly.
typed_data	✔️ All dependencies utilized correctly.

For details on how to fix these, see dependency_validator.

This check can be disabled by tagging the PR with skip-unused-dependencies-check.

Changelog Entry ❗

Package	Changed Files
package:args	pkgs/args/README.md
package:characters	pkgs/characters/README.md
package:collection	pkgs/collection/README.md
package:crypto	pkgs/crypto/README.md
package:logging	pkgs/logging/README.md
package:os_detect	pkgs/os_detect/README.md
package:path	pkgs/path/README.md
package:typed_data	pkgs/typed_data/README.md

Changes to files need to be accounted for in their respective changelogs.

This check can be disabled by tagging the PR with skip-changelog-check.

Coverage

⚠️

File	Coverage
pkgs/args/example/readme_examples.dart	💔 Not covered
pkgs/characters/example/readme_examples.dart	💔 Not covered
pkgs/collection/example/readme_examples.dart	💔 Not covered
pkgs/crypto/example/readme_examples.dart	💔 Not covered
pkgs/logging/example/readme_examples.dart	💔 Not covered
pkgs/os_detect/example/readme_examples.dart	💔 Not covered
pkgs/path/example/readme_examples.dart	💔 Not covered
pkgs/typed_data/example/readme_examples.dart	💔 Not covered

This check for test coverage is informational (issues shown here will not fail the PR).

This check can be disabled by tagging the PR with skip-coverage-check.

Breaking changes ✔️

Package	Change	Current Version	New Version	Needed Version	Looking good?
args	Breaking	2.7.0	2.8.0-wip	2.8.0-wip	✔️
characters	None	1.4.1	1.4.1	1.4.1	✔️
collection	Breaking	1.19.1	1.20.0-wip	1.20.0-wip	✔️
crypto	Non-Breaking	3.0.7	3.0.8-wip	3.0.8-wip	✔️
logging	None	1.3.0	1.3.1-wip	1.3.1-wip	✔️
os_detect	None	2.0.3	2.0.4-wip	2.0.4-wip	✔️
path	None	1.9.1	1.9.2-wip	1.9.2-wip	✔️
typed_data	None	1.4.0	1.4.1-wip	1.4.1-wip	✔️

This check can be disabled by tagging the PR with skip-breaking-check.

API leaks

⚠️

The following packages contain symbols visible in the public API, but not exported by the library. Export these symbols or remove them from your publicly visible API.

Package	Leaked API symbol	Leaking sources
path	InternalStyle	context.dart::Context::style

This check can be disabled by tagging the PR with skip-leaking-check.

lrhn

The formatting fixed are nice. Having the code being guaranteed to parse is nice.

There is no use of renaming that I don't find completely misplaced, and the majority of the RegExps are questionable.
I don't think RegExp-replace is a good feature to have. It encourages not handling the actual problems, just "fixing it in post-production", which is counter to the entire point of having the code in a døDart file. The example code can still be completely wrong after a replace.
That would go for literal text replacing too, being RegExp just makes it even more error prone. (Now you have two problems!)

I think one should be very explicit about which lints (including "modes") are enabled for the example code. It should be the lints of the reader, not the author, which would mean just the recommended lints.

Enforcing more than that may make you write thighs in the examples that users don't need.
(It may not matter, but if it does... It probably shouldn't.)

lrhn · 2026-06-22T05:31:01Z

 First create an [ArgParser][]:

-    var parser = ArgParser();
+<?code-excerpt "example/readme_examples.dart (parser-init)"?>


Is dartdoc officially specified to ignore HTML processing instructions?

We shouldn't be depending on accidental behavior.

(And I'd still prefer to use a dartdoc example directive over yet-another-tool... But I also like that that code is actually on the source, because I read source. So maybe dartdoc example directives should allow code to be inline as well, and then check that it matches the external region. @jonasfj )

lrhn · 2026-06-22T05:41:07Z

-      // ...
-      var results = parser.parse(args);
-    }
+<?code-excerpt "example/readme_examples.dart (parser-main-args)" replace="/void mainExample/main/g; /  final parser = ArgParser\(\);\n//g;"?>


Replacement functionality feels to miss the point.

The point of this feature is to check that the example code is being tested, by making sure it's the same as the code in the file that is being tested.

If you can replace arbitrary text using something as ungrammatical as a RegExp, then you've checked nothing.
If you could only rename a declaration, then maybe it's proof of something.
Of if you could only rename a main function, maybe that would still be enough.

(In this case it should not remove the void!)

lrhn · 2026-06-22T05:42:36Z


 By default, values for a multi-valued option may also be separated with commas:

+<?code-excerpt "example/readme_examples.dart (parser-multi-option-commas)" replace="/multiCommaParser/parser/g; /multiCommaResults/results/g;"?>


This rename should not be necessary. Just rename in the original.

lrhn · 2026-06-22T05:48:17Z

 from a range of possible commands. When parsing an argument list, you can then
 determine which command was entered and what options were provided for it.

+<?code-excerpt "example/readme_examples.dart (parser-command-results)" replace="/commandResults/results/g; /\?.name/.name/g; /\?\['all'\]/['all']/g;"?>


Same, rename should not be needed.

Also removing ? from ?. changes meaning significantly. I wouldn't assume the result is valid. Might as well just not use excerpts then.

(And the . should be escaped in the RegExp.
If the example had also contained test ? name : other, it would convert that to test .name : other.
If also put a \b after name do it doesn't match longer names.
People can't write RegExps correctly, having them anywhere near validation makes the outcome much more questionable. Replacements would be safer if they just replaced literal text, and didn't convert it to a RegExp first. And then, again, only entire identifiers which are declared variables, not, fx, text inside strings )

lrhn · 2026-06-22T05:58:20Z

+void main() {
+  // #docregion map-equality
+  const MapEquality<String, List<int>>(
+    keys: IdentityEquality<String>(),


I would not add the type arguments.

You don't need them if you don't have the strict dynamic "lint" enabled. It's not in the recommended set.

The classes are designed so that using no type arguments gives you a useful equality. It's what I would recommend, and it should work.

(We should perhaps change the bounds of the types to an explicit Object? I avoid strict dynamic issues.)

We should not necessarily test our example code with the same lintsx we use for our own code. If y use the recommended set, and not more than that. Example code is written to be used by people using the recommended lints.

lrhn · 2026-06-22T06:02:04Z

 ```dart
-import 'package:crypto/crypto.dart';
-import 'dart:convert'; // for the utf8.encode method
+var bytes = utf8.encode("foobar"); // data being hashed


Firmaet comments as statements (says the style guide).

// Data being hashed.

(It's OK to just comment what a print prints, but this is a phrase.)

lrhn · 2026-06-22T06:02:48Z

 when all the chunks have been added. The digest can then be retrieved
 from the `Sink<Digest>` used to create the input data sink.

+<?code-excerpt "example/readme_examples.dart (digest-chunked)" replace="/chunkedDigest/digest/g;"?>


Again, replace in the source, not here.

lrhn · 2026-06-22T06:05:54Z


 To use this package instead of `dart:io`, replace the import of `dart:io` with:

+<?code-excerpt "example/readme_examples.dart (import)" replace="/\/\/ ignore: unused_import\n//g;"?>


Can't you put the ignore outside the region?
If you can't, maybe is a dartdoc issue.

Are we sure the line ending won't be \r\n on some platforms?

lrhn · 2026-06-22T06:06:23Z

+// This file contains the code snippets for os_detect/README.md.
+
+// #docregion import
+// ignore: unused_import


Or just, like, use it!
Or use ignore-for-file.

lrhn · 2026-06-22T06:07:41Z

 If you want to work with paths for a specific platform regardless of the underlying platform that the program is running on, you can create a
 [Context] and give it an explicit [Style]:

+<?code-excerpt "example/readme_examples.dart (context)" replace="/p.Style/Style/g;"?>


Import or without prefix too, and don't have the prefix in the source.

Or keep the prefix. Every example of an import has as p.

(And the RegExp should escape the ..)

Can you use two excerpts for over example, so you could except both an import and a later function?

WIP: Trying out excerpts

8e25b2d

github-actions Bot added package:args package:crypto package:characters package:logging package:os_detect package:path package:typed_data package:collection labels May 7, 2026

kevmoo mentioned this pull request May 7, 2026

Add 'excerpter' package to the labs repository dart-lang/labs#325

Closed

lrhn reviewed Jun 22, 2026

View reviewed changes


		By default, values for a multi-valued option may also be separated with commas:

		<?code-excerpt "example/readme_examples.dart (parser-multi-option-commas)" replace="/multiCommaParser/parser/g; /multiCommaResults/results/g;"?>


		To use this package instead of `dart:io`, replace the import of `dart:io` with:

		<?code-excerpt "example/readme_examples.dart (import)" replace="/\/\/ ignore: unused_import\n//g;"?>

Uh oh!

Conversation

kevmoo commented May 7, 2026

Uh oh!

kevmoo commented May 7, 2026

1. collection: Non-functional Legacied MapEquality Constructor

2. logging: FutureOr Return Type Mismatch in catchError

3. args: Broken Command Runner Snippet Formatting

4. Strict Type Safety & Modern Nullability

Uh oh!

github-actions Bot commented May 7, 2026

PR Health

Uh oh!

lrhn left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

1. `collection`: Non-functional Legacied `MapEquality` Constructor

2. `logging`: `FutureOr` Return Type Mismatch in `catchError`

3. `args`: Broken Command Runner Snippet Formatting

lrhn left a comment •

edited

Loading