[DRAFT] Add convert-dot-shorthand skill and apply to examples#13181
[DRAFT] Add convert-dot-shorthand skill and apply to examples#13181
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a new convert-dot-shorthand skill, which is a great initiative for modernizing Dart code. The approach of using an aggressive script followed by a review is well-documented. My feedback focuses on refining the skill's documentation (SKILL.md) and the conversion script (apply_all.sh) based on the valuable findings from the test runs you've included. The main suggestions are to correct the documentation regarding Colors.* conversions, which are incorrectly classified as 'good', and to remove this faulty conversion from the script to improve its efficiency.
|
|
||
| find "$TARGET_DIR" -type f -name "*.dart" -print0 | while IFS= read -r -d '' file; do | ||
| # Colors.black26 -> .black26 | ||
| sed -i -E 's/Colors\.([a-zA-Z0-9_]+)/\.\1/g' "$file" |
There was a problem hiding this comment.
The rejection reports show that converting Colors.* to .* consistently leads to compilation errors. This is because the dot shorthand feature requires the static members to be on the contextual type (e.g., Color), but the color constants are on the Colors class.
To make this script more efficient and save the agent from having to revert these changes every time, this conversion rule should be removed. The script should not knowingly introduce errors.
| **Known "Good" Shorthands (Usually Keep):** | ||
| * `Colors.*` -> `.*` (It's almost always obvious a color is a color). | ||
| * `EdgeInsets.all` / `EdgeInsets.symmetric` -> `.all` / `.symmetric` (Context is extremely strong inside a `padding` or `margin` property). | ||
| * `Alignment.*` -> `.*` (Very clear inside an `alignment` property). | ||
|
|
||
| **Known "Bad" Shorthands (Always Revert):** | ||
| * *Please document rules here as we discover them during manual evaluation.* |
There was a problem hiding this comment.
The 'Readability Heuristics' section has some inaccuracies based on the findings in the rejection reports. Specifically, Colors.* conversions are listed as 'Good' but they cause compilation errors. Also, the reports identify that FontWeight shorthands for numeric weights are detrimental to readability, which would be a great addition to the 'Bad' shorthands list.
I suggest updating this section to reflect these learnings. This will make the skill documentation more accurate and useful.
| **Known "Good" Shorthands (Usually Keep):** | |
| * `Colors.*` -> `.*` (It's almost always obvious a color is a color). | |
| * `EdgeInsets.all` / `EdgeInsets.symmetric` -> `.all` / `.symmetric` (Context is extremely strong inside a `padding` or `margin` property). | |
| * `Alignment.*` -> `.*` (Very clear inside an `alignment` property). | |
| **Known "Bad" Shorthands (Always Revert):** | |
| * *Please document rules here as we discover them during manual evaluation.* | |
| **Known "Good" Shorthands (Usually Keep):** | |
| * `EdgeInsets.all` / `EdgeInsets.symmetric` -> `.all` / `.symmetric` (Context is extremely strong inside a `padding` or `margin` property). | |
| * `Alignment.*` -> `.*` (Very clear inside an `alignment` property). | |
| **Known "Bad" Shorthands (Always Revert):** | |
| * `Colors.*` -> `.*`: This conversion is incorrect because the `Color` class (the typical contextual type) does not have the same static members as the `Colors` class. This will cause `dot_shorthand_undefined_member` compiler errors. | |
| * `FontWeight.w*` -> `.*`: Numeric font weights like `.w600` lose their meaning without the `FontWeight` class name prefix, harming readability. |
|
Visit the preview URL for this PR (updated for commit a160aa8): https://flutter-docs-prod--pr13181-add-convert-dot-shorthand-s-sm578s2v.web.app |
This PR introduces a new
convert-dot-shorthandskill to automate Dart 3.10 shorthand conversions. It also applies the script to 5 example directories (data-and-backend,get-started,googleapis,state_mgmt,visual_debugging) and consolidates the resulting heuristic rejection reports intemp_shorthand_skill_test_runs/.