Skip to content

[go_router_builder] Support nested parentheses, groups, and lookaround assertions in path parameters - #12764

Open
MaximeRougieux wants to merge 3 commits into
flutter:mainfrom
MaximeRougieux:fix/go-router-builder-lookahead-path-params
Open

[go_router_builder] Support nested parentheses, groups, and lookaround assertions in path parameters#12764
MaximeRougieux wants to merge 3 commits into
flutter:mainfrom
MaximeRougieux:fix/go-router-builder-lookahead-path-params

Conversation

@MaximeRougieux

Copy link
Copy Markdown

See #12571 for details about the changes

Pre-Review Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools.
  • I read the [Tree Hygiene] page, which explains my responsibilities.
  • I read and followed the [relevant style guides] and ran [the auto-formatter].
  • I signed the [CLA].
  • The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
  • I [linked to at least one issue that this PR fixes] in the description above.
  • I followed [the version and CHANGELOG instructions], using [semantic versioning] and the [repository CHANGELOG style], or I have commented below to indicate which documented exception this PR falls under[^1].
  • I updated/added any relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or I have commented below to indicate which [test exemption] this PR falls under[^1].
  • All existing and new tests are passing.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request refactors path parameter parsing in path_utils.dart to support nested parentheses, grouping constructs, and lookahead assertions by replacing the previous regular expression with a custom scanning mechanism. It also adds corresponding unit tests and bumps the package version to 4.4.1. Feedback is provided regarding a missing import for package:collection/collection.dart which is required for the firstOrNull extension getter.

// found in the LICENSE file.

final RegExp _parameterRegExp = RegExp(r':(\w+)(\((?:\\.|[^\\()])+\))?');
final RegExp _parameterNameRegExp = RegExp(r':(\w+)');

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

The firstOrNull extension getter is used on line 33, but package:collection/collection.dart is not imported in this file. This will cause a compile-time error. Please add the import at the top of the file.

Suggested change
final RegExp _parameterNameRegExp = RegExp(r':(\w+)');
import 'package:collection/collection.dart';
final RegExp _parameterNameRegExp = RegExp(r':(\w+)');

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

1 participant