Repository navigation
[string_scanner] Extract LineScannerMixin and deduplicate eager scanning logic - #2569
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the line and column tracking logic by introducing a new LineScannerMixin on StringScanner. This mixin is then used by both LineScanner and EagerSpanScanner, successfully eliminating a significant amount of duplicated code. The feedback suggests adding a code example to the documentation of the newly introduced public LineScannerMixin to comply with the repository's style guide.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
@natebosch can you please review this patch? it is code cleanup |
|
@kevmoo Hi Kevin, could you please approve the workflow runs for CI when you get a chance? Thanks! |
|
Also introduces a new public mixin type. Which seems nice. |
Add `show` clauses to the public library exports to make the public API surface area explicit. Move the `EagerSpanScanner` which is not public into `line_spanner.dart` so the shared mixin can be private. Add a dependency on `package:meeta` and mark it as `@internal`. Remove some redundancy in docs. Remove the changelog since it's no longer user facing.
I disagree. I think we should hide it (so I did). |
Revisions updated by `dart tools/rev_sdk_deps.dart`. native (https://github.com/dart-lang/native/compare/f628b26..27f7678): 27f767827 Mon Sep 28 14:51:28 2026 +0200 Daco Harkes [hooks_runner] Remove `KernelAssets` and `Target` (dart-lang/native#3703) ddc7d6deb Mon Sep 28 05:40:03 2026 +0200 Jakob Kordež [ffigen] Skip typedefs to function pointers with unsupported parameter types (dart-lang/native#3694) 177d283d9 Mon Sep 28 04:30:28 2026 +0200 Jakob Kordež [ffigen] Generate bindings for C++ enums, structs and unions nested in namespaces and records (dart-lang/native#3644) 5ee8f67d8 Mon Sep 28 11:39:14 2026 +1000 Liam Appelbe [objective_c] Clarify block isolate scoping (dart-lang/native#3692) 25f090116 Fri Sep 25 12:09:12 2026 +1000 Liam Appelbe [ffigen][jnigen] Deprecate YAML config (dart-lang/native#3654) 8364413eb Fri Sep 25 10:09:50 2026 +1000 Liam Appelbe [jnigen] Document nullability (dart-lang/native#3690) e8d518574 Thu Sep 24 18:02:46 2026 +1000 Liam Appelbe [ffigen] Code snippet-ify ffigen and objective_c (dart-lang/native#3685) 663385955 Thu Sep 24 03:21:53 2026 +0300 Manar Elhabbal [ffigen] prevent setting both cpp and objectiveC in ffigenerator (dart-lang/native#3687) 1cf40952a Thu Sep 24 09:27:09 2026 +1000 Liam Appelbe [objective_c] Remove arm64e build hack (dart-lang/native#3680) c44550b6e Wed Sep 23 13:01:33 2026 +0200 Simon Binder [web_assets] Scaffold package (dart-lang/native#3610) 57b4bad69 Wed Sep 23 16:21:27 2026 +1000 Liam Appelbe [swift2objc] Code snippet-ify swift2objc (dart-lang/native#3684) 5cee1d0be Wed Sep 23 10:16:29 2026 +0530 Rachit Vikram Singh [ffigen] Add Input.appendCompilerOptions to append clang args to defaults (`#3630`) (dart-lang/native#3676) 225a648df Wed Sep 23 14:37:13 2026 +1000 Liam Appelbe [jnigen] Code snippet-ify jnigen and co (dart-lang/native#3641) 2030decf9 Wed Sep 23 13:51:24 2026 +1000 Liam Appelbe [infra] Move snippets_tool to its own package (dart-lang/native#3683) 6fa2beea2 Wed Sep 23 12:14:28 2026 +1000 Liam Appelbe [objective_c] Revert `#3664` (dart-lang/native#3681) ed7605f8a Tue Sep 22 12:03:22 2026 +0530 Sivapriya [jni] Add deep conversion utils (dart-lang/native#3635) 8574b7422 Tue Sep 22 08:31:11 2026 +0200 Lukas Klingsbo [ffigen] Allow package:cli_util 0.5.x and 0.6.x (dart-lang/native#3673) test (https://github.com/dart-lang/test/compare/9946c95..4f92d53): 4f92d530 Fri Sep 25 16:46:22 2026 -0700 Nate Bosch Unskip a test on windows (dart-lang/test#2772) 7fb84866 Fri Sep 25 14:52:39 2026 -0700 Nate Bosch Bundle skips at the end of the GitHub reporter (dart-lang/test#2768) 603e5ec5 Thu Sep 24 14:00:41 2026 -0700 Nate Bosch Clean up the runner's temporary directories (dart-lang/test#2763) 4bf04d90 Thu Sep 24 12:41:15 2026 -0700 Nate Bosch Refactor signal handling in executable.dart (dart-lang/test#2765) 2c660a84 Tue Sep 22 15:33:34 2026 -0700 Nate Bosch Regenerate mono_repo output (dart-lang/test#2767) tools (https://github.com/dart-lang/tools/compare/3614ce0..d87eaf7): d87eaf79 Thu Sep 24 08:31:33 2026 -0700 Kevin Moore fix(benchmark_harness): migrate subprocess spawning to package:cli_util (dart-lang/tools#2585) 8ccf8ccf Wed Sep 23 05:09:10 2026 +0530 Aman Maurya [string_scanner] Extract a mixin for LineScanner and deduplicate eager scanning logic (dart-lang/tools#2569) afcdd6d9 Tue Sep 22 15:38:07 2026 +0200 Morgan :) Document that watch events converge on filesystem state. (dart-lang/tools#2607) f2157cbc Tue Sep 22 04:52:58 2026 +0300 Hasan Armoush [code_builder] Add null-aware cascades, cascade index, and fluent cascade helpers (dart-lang/tools#2540) web (https://github.com/dart-lang/web/compare/e5c6c02..ec71a4c): ec71a4c Mon Sep 28 10:07:01 2026 -0700 Kevin Moore [js_interop_gen] Require `code_builder: ^4.12.0` and update golden headers (dart-lang/web#576) [email protected] Change-Id: I766bc5b2b1297e3251990821bb73387ba685e39b Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/556320 Commit-Queue: Nate Bosch <[email protected]> Reviewed-by: Brian Quinlan <[email protected]>
Description
This PR refactors
LineScannerandEagerSpanScannerinpackage:string_scannerto share eager line and column tracking implementation viaLineScannerMixin:position=setter logic,_adjustLineAndColumn,scanChar,readChar, andscanintomixin LineScannerMixin on StringScanner.EagerSpanScannerto usewith LineScannerMixin, eliminating ~120 lines of duplicate regex-based state tracking and removing the legacyTODO(nweiz)trackingsdk#23770.pkgs/string_scanner/CHANGELOG.md.Verification
dart analyze: 0 issues found.dart test: All 185 tests passed acrossline_scanner,span_scanner,string_scanner, anderror_test.dart format: Code formatted according to Dart style guide.Contribution guidelines:
dart format.Many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.
Note: The Dart team is trialing Gemini Code Assist. Don't take its comments as final Dart team feedback. Use the suggestions if they're helpful; otherwise, wait for a human reviewer.