-
Notifications
You must be signed in to change notification settings - Fork 29.5k
In "flutter create", use the project directory in the suggested "cd" command instead of the main source file path #173132
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.
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
Conversation
…command instead of the main source file path
There was a problem hiding this comment.
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 correctly changes the suggested cd command in flutter create to use the project directory path instead of the main source file path. The accompanying test change is also an improvement. I've suggested a small refinement to the test to make it more robust.
| RegExp cdLine = RegExp(r'^ \$ cd .*flutter_project$', multiLine: true); | ||
| expect(cdLine.hasMatch(logger.statusText), isTrue); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a good improvement to make the test more specific. To make it even more robust and less reliant on hardcoded strings, you could construct the expected path from the projectDir variable. This avoids hardcoding flutter_project and ensures the test correctly handles any changes to the test setup or relative path generation.1
final String expectedPath = globals.fs.path.relative(projectDir.path);
final RegExp cdLine = RegExp(r'^ \$ cd ' + RegExp.escape(expectedPath) + r'$', multiLine: true);
expect(cdLine.hasMatch(logger.statusText), isTrue, reason: 'statusText does not contain "cd $expectedPath"');
Style Guide References
Footnotes
|
If this made it into 3.35 we should probably cherrypick it (to beta) after merging. |
|
This issue does not affect the 3.35 branch. |
…sted "cd" command instead of the main source file path (flutter/flutter#173132)
…sted "cd" command instead of the main source file path (flutter/flutter#173132)
…sted "cd" command instead of the main source file path (flutter/flutter#173132)
…sted "cd" command instead of the main source file path (flutter/flutter#173132)
…sted "cd" command instead of the main source file path (flutter/flutter#173132)
…command instead of the main source file path (flutter#173132)
…command instead of the main source file path (flutter#173132)
…command instead of the main source file path (flutter#173132)
…command instead of the main source file path (flutter#173132)
…sted "cd" command instead of the main source file path (flutter/flutter#173132)
No description provided.