Homepage "compute pi" example: Dart sources, tests, and HTML-version generator #448

chalin · 2017-11-30T17:51:23Z

Contributes to #407, by

Creating a testable version of the homepage "compute pi" example.
Providing tests for the "compute pi" example.
Providing a script for generating the HTML version of the homepage example, which contains embedded tootlips.

Specifically, some of the files in this PR are:

examples/lib/pi_monte_carlo.dart:
source for the "compute pi" homepage example. This was derived from the DartPad sources.
examples/lib/pi_monte_carlo_tooltips.html:
tooltips for the highlighted features of the homepage example.
scripts/create_site_main_example.dart:
simple script used to generate the HTML version of the homepage example from the Dart source and tooltip files.
examples/test/pi_test.dart:
tests the homepage example.

I've ensured that the generated HTML version of the homepage example matches the current version visible on dartlag.org.

This PR also includes some minor CSS changes to modernize the look of the tooltip/footnote area of the homepage. These are in a separate commit in case you prefer the old look.

Staged at: https://dartlang-org-staging-0.firebaseapp.com

filiph

Thanks for doing this! In retrospect, it was kind of stupid of me to have this huge example code featured on the front page and have no continuous testing set up for it. Kudos for fixing that glaring bug.

I'm not a big fan of making the "tooltip" more visible via color. It's now too high in the visual hierarchy of the page.

Can you please revert that change and stash it? We don't need it for testing/generating the example. We might want to noodle on it in a separate PR.

filiph · 2017-11-30T18:52:07Z

scripts/create_site_main_example.dart

+      line = lineWithoutTipInstruction.replaceFirst(escapedAnchor,
+          '<span class="frontpage-highlight" data-text="$ttText">$escapedAnchor</span>');
+    }
+    // TODO: remove these temporary conversions used to minimize diffs with existing HTML


If these are just to make the diff prettier, I think you can remove it. If the difference is functional, keep these.

With these in place, the generation script creates exactly the same HTML (when compared to the file as created in the second commit). I'll strip these out in a followup commit, if that is ok with you.

Actually, I've removed them in a separate commit.

filiph · 2017-11-30T18:54:50Z

scripts/create_site_main_example.dart

+  final HtmlEscape _htmlEscape =
+      new HtmlEscape(new HtmlEscapeMode(escapeLtGt: true));
+  final HtmlUnescape _htmlUnescape = new HtmlUnescape();
+  final tipRE = new RegExp(r'^(.*?) //!tip\("([^"]+)"\)$');


nit: maybe tipPattern? When I see tipRE in code I don't know what the "RE" means (maybe it's just me being more used to "RegEx").

Renamed to tipRegExp.

filiph · 2017-11-30T18:56:32Z

scripts/create_site_main_example.dart

+      if (match == null) throw 'Tip instruction match failed for line: $line';
+      final lineWithoutTipInstruction = match[1],
+          anchor = _htmlUnescape.convert(match[2]),
+          tooltip = tooltips[indexOfNextTooltip++],


nit: do indexOfNextTooltip += 1; separately. The ++ is really easy to overlook.

filiph · 2017-11-30T18:58:27Z

scripts/create_site_main_example.dart

+      final lineWithoutTipInstruction = match[1],
+          anchor = _htmlUnescape.convert(match[2]),
+          tooltip = tooltips[indexOfNextTooltip++],
+          ttAnchor = tooltip[0],


tooltipAnchor ?

filiph · 2017-11-30T19:01:42Z

src/_assets/javascripts/main.js

      footnote.removeClass('blink');
-    }, 1000);
+    }, 2000);


Why the increased time?

It just made more sense with the new look. Since the commit that this change is a part of has been stashed, it will go away (and we can discuss it later, if need be).

filiph · 2017-11-30T19:09:05Z

examples/lib/pi_monte_carlo.dart

+}
+
+/// Generates a stream of increasingly accurate estimates of π. //!tip("///")
+Stream<double> computePi({int batch: 100000}) async* { //!tip("async*") //!tip("{int batch: 100000}") //!tip("<double>")


Was the change from batch of 1M to 100K intentional? No problem either way, just checking.

This was to match the original sources in the published DartPad: https://dartpad.dartlang.org/fb763a4a770b5cdd896982e10ccf4118

- Changed single quotes to double quotes for a `data-text` attribute. - Make default batch value match what is in the current DartPad (drop one 0).

…s compatibility

chalin · 2017-11-30T19:38:58Z

Can you please revert that change and stash it? We don't need it for testing/generating the example. We might want to noodle on it in a separate PR.

Done

And the staging server has been refreshed: https://dartlang-org-staging-0.firebaseapp.com

filiph · 2017-11-30T19:47:53Z

LGTM!

chalin · 2017-11-30T23:13:14Z

Thanks for doing this! In retrospect, it was kind of stupid of me to have this huge example code featured on the front page and have no continuous testing set up for it. Kudos for fixing that glaring bug.

Oh, and thanks for the kudos. Double kudos to you for such a great example, covering many of Dart's features in such a compact example!

chalin added fix.examples Adds or changes example infra.structure Relates to the tools that create dart.dev test.general Relates to unit, integration, perf testing labels Nov 30, 2017

chalin added this to the 1.24 milestone Nov 30, 2017

chalin requested a review from filiph November 30, 2017 17:51

googlebot added the cla: yes Contributor has signed the Contributor License Agreement label Nov 30, 2017

chalin mentioned this pull request Nov 30, 2017

Code excerpts should be auto-updated and tested #407

Closed

20 tasks

filiph suggested changes Nov 30, 2017

View reviewed changes

chalin added 7 commits November 30, 2017 14:20

Move main example HTML into its own file, unchanged

2e1f4a6

Main example HTML: minor fixes

5c2c1c7

- Changed single quotes to double quotes for a `data-text` attribute. - Make default batch value match what is in the current DartPad (drop one 0).

Main example source + script to generate tooltip annotated version

717f36b

Adjust dartfmt.sh script

d6e116f

Post-review updates

0f56819

Remove temporary script conversions used by script to ensure backward…

e88adcc

…s compatibility

The new generated main example in HTML

0d06f29

chalin force-pushed the chalin-main-site-ex-1130 branch from eafcbf4 to 0d06f29 Compare November 30, 2017 19:38

filiph approved these changes Nov 30, 2017

View reviewed changes

chalin merged commit 57a8f86 into dart-lang:master Nov 30, 2017

chalin deleted the chalin-main-site-ex-1130 branch November 30, 2017 19:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Homepage "compute pi" example: Dart sources, tests, and HTML-version generator #448

Homepage "compute pi" example: Dart sources, tests, and HTML-version generator #448

chalin commented Nov 30, 2017

filiph left a comment

filiph Nov 30, 2017

chalin Nov 30, 2017

chalin Nov 30, 2017

filiph Nov 30, 2017

chalin Nov 30, 2017

filiph Nov 30, 2017

chalin Nov 30, 2017

filiph Nov 30, 2017

chalin Nov 30, 2017

filiph Nov 30, 2017

chalin Nov 30, 2017

filiph Nov 30, 2017

chalin Nov 30, 2017

chalin commented Nov 30, 2017

filiph commented Nov 30, 2017

chalin commented Nov 30, 2017

Homepage "compute pi" example: Dart sources, tests, and HTML-version generator #448

Homepage "compute pi" example: Dart sources, tests, and HTML-version generator #448

Conversation

chalin commented Nov 30, 2017

filiph left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chalin commented Nov 30, 2017

filiph commented Nov 30, 2017

chalin commented Nov 30, 2017