Skip to content

[flutter_plugin_tools] Reimplements the excerpt system inline in the tool, rather than relying on a separate package. - #4417

Merged
auto-submit[bot] merged 1 commit into
flutter:mainfrom
Hixie:excerpt_2
Jul 11, 2023
Merged

[flutter_plugin_tools] Reimplements the excerpt system inline in the tool, rather than relying on a separate package.#4417
auto-submit[bot] merged 1 commit into
flutter:mainfrom
Hixie:excerpt_2

Conversation

@Hixie

Copy link
Copy Markdown
Contributor
  • Allows excerpts to come from any package, not just examples.
  • Fixes a bug in the excerpting logic that was causing a stray } to appear in one example.
  • Removes the need for build.excerpt.yaml files.
  • Remove the dependency on build_runner for excerpts.
  • Reduces the time to generate the excerpts from about 10 minutes to about 5 seconds.
  • Almost certainly fixes[plugins] Obscure failure when using update-excerpts while IDE is open flutter#107180 (untested).

The new logic is not quite backwards compatible; the path-base feature now specifies a real path to the actual source directories, rather than a path into the invisible generated excerpts/ directory with its special structure. Also, a number of features from the previous package that were not actually used in this repository are no longer supported (such as having multiple section names per #docregion pragma).

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use dart format.)
  • 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 listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.
  • I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

@Hixie

Copy link
Copy Markdown
ContributorAuthor

@Hixie
Hixieforce-pushed the excerpt_2 branch 2 times, most recently from 01dfdcd to f026326CompareJuly 10, 2023 05:29
@Hixie

Copy link
Copy Markdown
ContributorAuthor
  • Reduces the time to generate the excerpts from about 10 minutes to about 5 seconds.

O.o

Any idea why the package is so slow? Is there something that we should fix upstream to improve the website repo CI?

Well, the old code using build_runner, the Dart compiler, codegen, stored all the extracts on disk, and had two separate processes for extracting and injecting. The new code does none of that. Not running code is faster than running code.

I'm not sure if it makes sense to upstream this because I don't fully understand the website repo's use case so it's hard to say whether this code addresses the same needs.

@Hixie

Copy link
Copy Markdown
ContributorAuthor

Proposed new documentation for the wiki:

Nice! A few comments, but generally looks good.

Added a bit about path-base's default.

@Hixie

Copy link
Copy Markdown
ContributorAuthor

updated, PTAL.

@tarrinnealtarrinneal 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.

This is a welcome change, thank you!

@Hixie
Hixieforce-pushed the excerpt_2 branch 4 times, most recently from 248fc71 to 81b0b51CompareJuly 11, 2023 00:23
@HixieHixie mentioned this pull request Jul 11, 2023
11 tasks

@stuartmorgan-gstuartmorgan-g left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, thanks!

I saw from your comments in the rfw PR that we may need to iterate more for that specific case, but I think we should land this as-is and then iterate from there in follow-ups since this is already a big quality of life improvement over the current implementation.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I would vote for removing things we have no intention of ever using (scss and ts).

@Hixie

Copy link
Copy Markdown
ContributorAuthor

I will remove scss and ts and land it. Thanks for the reviews!

…ng on a separate package.
* Allows excerpts to come from any package, not just examples.
* Fixes a bug in the excerpting logic that was causing a stray `}` to appear in one example.
* Removes the need for `build.excerpt.yaml` files.
* Remove the dependency on build_runner for excerpts.
* Reduces the time to generate the excerpts from about 10 minutes to about 5 seconds.
The new logic is not quite backwards compatible; the `path-base` feature now specifies a real path to the actual source directories, rather than a path into the invisible generated `excerpts/` directory with its special structure. Also, a number of features from the previous package that were not actually used in this repository are no longer supported (such as having multiple section names per `#docregion` pragma).
@HixieHixie added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 11, 2023
@auto-submit
auto-submitBot merged commit bd5d191 into flutter:mainJul 11, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 12, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 12, 2023
auto-submitBot pushed a commit to flutter/flutter that referenced this pull request Jul 12, 2023
flutter/packages@188a846...2508714
2023-07-12 39314023+wer-mathurin@users.noreply.github.com ADD appBarBreakpoint (flutter/packages#4434)
2023-07-12 engine-flutter-autoroll@skia.org Roll Flutter from 65ff3cb to 3ec96a8 (5 revisions) (flutter/packages#4415)
2023-07-12 tarrinneal@gmail.com [image_picker] Roll dependancies to avoid error (flutter/packages#4431)
2023-07-12 49699333+dependabot[bot]@users.noreply.github.com [in_app_pur]: Bump com.android.billingclient:billing from 6.0.0 to 6.0.1 in /packages/in_app_purchase/in_app_purchase_android/android (flutter/packages#4422)
2023-07-12 ditman@gmail.com [file_selector] Avoids using path_provider in web example app. (flutter/packages#4445)
2023-07-12 ian@hixie.ch [rfw] Add some more documentation for RFW (flutter/packages#4349)
2023-07-12 stuartmorgan@google.com [ci] Enable LUCI legacy analysis (flutter/packages#4435)
2023-07-11 ian@hixie.ch [webview_flutter_wkwebview] NSError.toString (flutter/packages#4441)
2023-07-11 stuartmorgan@google.com [ci] Remove unused Chromium setup (flutter/packages#4437)
2023-07-11 ian@hixie.ch [flutter_plugin_tools] Reimplements the excerpt system inline in the tool, rather than relying on a separate package. (flutter/packages#4417)
2023-07-11 10687576+bparrishMines@users.noreply.github.com [ci] Remove webview_flutter implementation opt outs for custom analysis (flutter/packages#4438)
2023-07-11 stuartmorgan@google.com [palette_generator] Add web support to unit tests (flutter/packages#4440)
2023-07-11 stuartmorgan@google.com [tool] Conditionalize color on `stdout` (flutter/packages#4436)
2023-07-11 47866232+chunhtai@users.noreply.github.com [go_router_builder] Cleans up builder code. (flutter/packages#4356)
If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC flutter-ecosystem@google.com,rmistry@google.com on the revert to ensure that a human
is aware of the problem.
To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose
To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug
Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
LouiseHsu pushed a commit to LouiseHsu/flutter that referenced this pull request Jul 13, 2023
flutter/packages@188a846...2508714
2023-07-12 39314023+wer-mathurin@users.noreply.github.com ADD appBarBreakpoint (flutter/packages#4434)
2023-07-12 engine-flutter-autoroll@skia.org Roll Flutter from 65ff3cb to 3ec96a8 (5 revisions) (flutter/packages#4415)
2023-07-12 tarrinneal@gmail.com [image_picker] Roll dependancies to avoid error (flutter/packages#4431)
2023-07-12 49699333+dependabot[bot]@users.noreply.github.com [in_app_pur]: Bump com.android.billingclient:billing from 6.0.0 to 6.0.1 in /packages/in_app_purchase/in_app_purchase_android/android (flutter/packages#4422)
2023-07-12 ditman@gmail.com [file_selector] Avoids using path_provider in web example app. (flutter/packages#4445)
2023-07-12 ian@hixie.ch [rfw] Add some more documentation for RFW (flutter/packages#4349)
2023-07-12 stuartmorgan@google.com [ci] Enable LUCI legacy analysis (flutter/packages#4435)
2023-07-11 ian@hixie.ch [webview_flutter_wkwebview] NSError.toString (flutter/packages#4441)
2023-07-11 stuartmorgan@google.com [ci] Remove unused Chromium setup (flutter/packages#4437)
2023-07-11 ian@hixie.ch [flutter_plugin_tools] Reimplements the excerpt system inline in the tool, rather than relying on a separate package. (flutter/packages#4417)
2023-07-11 10687576+bparrishMines@users.noreply.github.com [ci] Remove webview_flutter implementation opt outs for custom analysis (flutter/packages#4438)
2023-07-11 stuartmorgan@google.com [palette_generator] Add web support to unit tests (flutter/packages#4440)
2023-07-11 stuartmorgan@google.com [tool] Conditionalize color on `stdout` (flutter/packages#4436)
2023-07-11 47866232+chunhtai@users.noreply.github.com [go_router_builder] Cleans up builder code. (flutter/packages#4356)
If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC flutter-ecosystem@google.com,rmistry@google.com on the revert to ensure that a human
is aware of the problem.
To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose
To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug
Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
@reidbakerreidbaker mentioned this pull request Aug 4, 2023
11 tasks
creatorpiyush pushed a commit to creatorpiyush/packages that referenced this pull request Jun 10, 2026
…tool, rather than relying on a separate package. (flutter#4417)
* Allows excerpts to come from any package, not just examples.
* Fixes a bug in the excerpting logic that was causing a stray `}` to appear in one example.
* Removes the need for `build.excerpt.yaml` files.
* Remove the dependency on build_runner for excerpts.
* Reduces the time to generate the excerpts from about 10 minutes to about 5 seconds.
* Almost certainly fixesflutter/flutter#107180 (untested).
The new logic is not quite backwards compatible; the `path-base` feature now specifies a real path to the actual source directories, rather than a path into the invisible generated `excerpts/` directory with its special structure. Also, a number of features from the previous package that were not actually used in this repository are no longer supported (such as having multiple section names per `#docregion` pragma).
bisor0627 pushed a commit to bisor0627/packages that referenced this pull request Jun 19, 2026
…tool, rather than relying on a separate package. (flutter#4417)
* Allows excerpts to come from any package, not just examples.
* Fixes a bug in the excerpting logic that was causing a stray `}` to appear in one example.
* Removes the need for `build.excerpt.yaml` files.
* Remove the dependency on build_runner for excerpts.
* Reduces the time to generate the excerpts from about 10 minutes to about 5 seconds.
* Almost certainly fixesflutter/flutter#107180 (untested).
The new logic is not quite backwards compatible; the `path-base` feature now specifies a real path to the actual source directories, rather than a path into the invisible generated `excerpts/` directory with its special structure. Also, a number of features from the previous package that were not actually used in this repository are no longer supported (such as having multiple section names per `#docregion` pragma).
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[plugins] Obscure failure when using update-excerpts while IDE is open

5 participants

@Hixie@stuartmorgan-g@domesticmouse@reidbaker@tarrinneal