Refactoring two pieces of polymer:

Refactoring two pieces of polymer:
- loader logic: to make it easy to remove the mirror's loading code
- script compactor: to make it flatter and easier to extend later.

R=jmesserly@google.com

Committed: https://code.google.com/p/dart/source/detail?r=33403

[Siggi Cherem (dart-lang), 2014-03-06 22:45:22]

this is dull work just to make some of the future changes easier to review.

https://codereview.chromium.org/189213003/diff/1/pkg/polymer/lib/src/build/sc...
File pkg/polymer/lib/src/build/script_compactor.dart (left):

https://codereview.chromium.org/189213003/diff/1/pkg/polymer/lib/src/build/sc...
pkg/polymer/lib/src/build/script_compactor.dart:86: .where((url) => url !=
null).toList();
one subtle, probably not relevant, difference: this filter made it so that we
generated a  prefix only for valid urls. After the refactor, we generate
prefixes for all urls, but only use those that are valid. The difference in the
generated code might be that some files will have imports like:
  import 'foo.dart' as i0;
  import 'bar.dart' as i1;
  import 'baz.dart' as i4; // i2 and i3 were skipped.

https://codereview.chromium.org/189213003/diff/1/pkg/polymer/lib/src/build/sc...
File pkg/polymer/lib/src/build/script_compactor.dart (right):

https://codereview.chromium.org/189213003/diff/1/pkg/polymer/lib/src/build/sc...
pkg/polymer/lib/src/build/script_compactor.dart:6: library
polymer.src.build.script_compactor;
there is no new logic on this file, just code refactoring

https://codereview.chromium.org/189213003/diff/1/pkg/polymer/lib/src/build/sc...
pkg/polymer/lib/src/build/script_compactor.dart:165: Future<List<_Initializer>>
_initializersOf(AssetId dartLibrary) {
I considered fixing more below, but I might be changing this anyways to use the
resolver, so I'll wait to do this part until later.

https://codereview.chromium.org/189213003/diff/1/pkg/polymer/lib/src/instance...
File pkg/polymer/lib/src/instance.dart (right):

https://codereview.chromium.org/189213003/diff/1/pkg/polymer/lib/src/instance...
pkg/polymer/lib/src/instance.dart:654: bindable.value = smoke.read(this, name);
oops - one lingering use of mirrors...

[Siggi Cherem (dart-lang), 2014-03-06 22:51:52]

seems like I forgot to fix an import after the merge. Done.

[Jennifer Messerly, 2014-03-06 22:54:16]

LGTM!!

https://codereview.chromium.org/189213003/diff/1/pkg/polymer/lib/src/build/mi...
File pkg/polymer/lib/src/build/mirrors_remover.dart (right):

https://codereview.chromium.org/189213003/diff/1/pkg/polymer/lib/src/build/mi...
pkg/polymer/lib/src/build/mirrors_remover.dart:33: .replaceAll('mirror_loader',
'static_loader')):
maybe double check that 'mirror_loader' was found too?

https://codereview.chromium.org/189213003/diff/1/pkg/polymer/lib/src/build/sc...
File pkg/polymer/lib/src/build/script_compactor.dart (right):

https://codereview.chromium.org/189213003/diff/1/pkg/polymer/lib/src/build/sc...
pkg/polymer/lib/src/build/script_compactor.dart:44: class _ScriptCompactor
extends PolymerTransformer {
wow, this is so much more readable

https://codereview.chromium.org/189213003/diff/1/pkg/polymer/lib/src/build/sc...
pkg/polymer/lib/src/build/script_compactor.dart:78: .then((libraryIds) {
IIRC when I asked the Pub folks, the way they do this is +4 on this line but
leave the other line below unchanged (i.e. +2 indent like you have). Kind of
like how you'd wrap method arguments.

(although it reads fine to me like this too.)

[Siggi Cherem (dart-lang), 2014-03-06 23:09:58]

thanks!

https://codereview.chromium.org/189213003/diff/1/pkg/polymer/lib/src/build/mi...
File pkg/polymer/lib/src/build/mirrors_remover.dart (right):

https://codereview.chromium.org/189213003/diff/1/pkg/polymer/lib/src/build/mi...
pkg/polymer/lib/src/build/mirrors_remover.dart:33: .replaceAll('mirror_loader',
'static_loader')):
On 2014/03/06 22:54:17, John Messerly wrote:
> maybe double check that 'mirror_loader' was found too?

Done.

https://codereview.chromium.org/189213003/diff/1/pkg/polymer/lib/src/build/sc...
File pkg/polymer/lib/src/build/script_compactor.dart (right):

https://codereview.chromium.org/189213003/diff/1/pkg/polymer/lib/src/build/sc...
pkg/polymer/lib/src/build/script_compactor.dart:78: .then((libraryIds) {
On 2014/03/06 22:54:17, John Messerly wrote:
> IIRC when I asked the Pub folks, the way they do this is +4 on this line but
> leave the other line below unchanged (i.e. +2 indent like you have). Kind of
> like how you'd wrap method arguments.
> 
> (although it reads fine to me like this too.)

interesting. I might prefer the style I had here, but I'll give it a shot.

[Siggi Cherem (dart-lang), 2014-03-06 23:23:32]

Committed patchset #2 manually as r33403 (presubmit successful).