support multiple entry points and only html entry points

support multiple entry points and only html entry points

R=sigmund@google.com

Committed: https://github.com/dart-lang/static_init/commit/8ff3cf318c18345a5ac9b99e6c315313c11893f6

[jakemac, 2015-02-09 22:10:39]

jakemac@google.com changed reviewers:
+ sigmund@google.com

[Siggi Cherem (dart-lang), 2015-02-09 22:55:18]

lgtm

https://codereview.chromium.org/906413002/diff/60001/CHANGELOG.md
File CHANGELOG.md (right):

https://codereview.chromium.org/906413002/diff/60001/CHANGELOG.md#newcode6
CHANGELOG.md:6: just be the original name but `.dart` will be replaced with
`.initialize.dart`.
to support our test infrastructure, maybe we want to provide a pubspec option
that specifies what suffix to add?

https://codereview.chromium.org/906413002/diff/60001/lib/transformer.dart
File lib/transformer.dart (right):

https://codereview.chromium.org/906413002/diff/60001/lib/transformer.dart#new...
lib/transformer.dart:70: return new InitializeTransformer(entryPoints);
nit: maybe combine everything in a single line and use `=>`?

https://codereview.chromium.org/906413002/diff/60001/lib/transformer.dart#new...
lib/transformer.dart:102: primaryId.path.replaceFirst('.dart',
'.initialize.dart'));
replaceLast? Or maybe path.replaceExtension()?

https://codereview.chromium.org/906413002/diff/60001/lib/transformer.dart#new...
lib/transformer.dart:127: transform.logger, script.sourceSpan) ==
originalDartFile)
not sure I like what the formatter does here. Might be worth adding {} to make
it better, or name this as a local function

https://codereview.chromium.org/906413002/diff/60001/lib/transformer.dart#new...
lib/transformer.dart:137: _relativePath(entryPoint.path, newDartFile.path);
it might just fine to inline this function here

https://codereview.chromium.org/906413002/diff/60001/pubspec.yaml
File pubspec.yaml (right):

https://codereview.chromium.org/906413002/diff/60001/pubspec.yaml#newcode27
pubspec.yaml:27: - test/init_method_test.dart
I believe you can also do:
$include:
 - test/initializer_test.dart
 - ...

[jakemac, 2015-02-09 23:40:04]

New patchsets have been uploaded after l-g-t-m from sigmund@google.com

[jakemac, 2015-02-09 23:41:05]

https://codereview.chromium.org/906413002/diff/60001/CHANGELOG.md
File CHANGELOG.md (right):

https://codereview.chromium.org/906413002/diff/60001/CHANGELOG.md#newcode6
CHANGELOG.md:6: just be the original name but `.dart` will be replaced with
`.initialize.dart`.
On 2015/02/09 22:55:17, Siggi Cherem (dart-lang) wrote:
> to support our test infrastructure, maybe we want to provide a pubspec option
> that specifies what suffix to add?

Ya, I think that can be added in the next release though.

https://codereview.chromium.org/906413002/diff/60001/lib/transformer.dart
File lib/transformer.dart (right):

https://codereview.chromium.org/906413002/diff/60001/lib/transformer.dart#new...
lib/transformer.dart:70: return new InitializeTransformer(entryPoints);
On 2015/02/09 22:55:17, Siggi Cherem (dart-lang) wrote:
> nit: maybe combine everything in a single line and use `=>`?

Done.

https://codereview.chromium.org/906413002/diff/60001/lib/transformer.dart#new...
lib/transformer.dart:102: primaryId.path.replaceFirst('.dart',
'.initialize.dart'));
On 2015/02/09 22:55:17, Siggi Cherem (dart-lang) wrote:
> replaceLast? Or maybe path.replaceExtension()?

unfortunately I don't see a good api for this but I did switch to using
path.url.withoutExtension in and just adding `.initialize.dart` to that.

https://codereview.chromium.org/906413002/diff/60001/lib/transformer.dart#new...
lib/transformer.dart:127: transform.logger, script.sourceSpan) ==
originalDartFile)
On 2015/02/09 22:55:17, Siggi Cherem (dart-lang) wrote:
> not sure I like what the formatter does here. Might be worth adding {} to make
> it better, or name this as a local function

I agree that its not ideal, but I don't really want to write code just to
satisfy the formatter either. I am not really a fan of local functions in
general, and adding {} actually makes it worse because of
https://github.com/dart-lang/dart_style/issues/145

https://codereview.chromium.org/906413002/diff/60001/lib/transformer.dart#new...
lib/transformer.dart:137: _relativePath(entryPoint.path, newDartFile.path);
On 2015/02/09 22:55:17, Siggi Cherem (dart-lang) wrote:
> it might just fine to inline this function here

ya good point originally I was using it more than once but now that I am not its
kind of useless :)

https://codereview.chromium.org/906413002/diff/60001/pubspec.yaml
File pubspec.yaml (right):

https://codereview.chromium.org/906413002/diff/60001/pubspec.yaml#newcode27
pubspec.yaml:27: - test/init_method_test.dart
On 2015/02/09 22:55:17, Siggi Cherem (dart-lang) wrote:
> I believe you can also do:
> $include:
>  - test/initializer_test.dart
>  - ...

Hmm, should we do that for all transformers then? This is nice in that its
similar to the polymer and up-coming web_components transformers, but I don't
have a strong opinion. 

Does $include support glob syntax?

[Siggi Cherem (dart-lang), 2015-02-10 01:45:48]

lgtm

https://codereview.chromium.org/906413002/diff/60001/pubspec.yaml
File pubspec.yaml (right):

https://codereview.chromium.org/906413002/diff/60001/pubspec.yaml#newcode27
pubspec.yaml:27: - test/init_method_test.dart
On 2015/02/09 23:41:05, jakemac wrote:
> On 2015/02/09 22:55:17, Siggi Cherem (dart-lang) wrote:
> > I believe you can also do:
> > $include:
> >  - test/initializer_test.dart
> >  - ...
> 
> Hmm, should we do that for all transformers then? This is nice in that its
> similar to the polymer and up-coming web_components transformers, but I don't
> have a strong opinion. 
> 
> Does $include support glob syntax?

it's mainly for iteration speed, so not entirely necessary. Up to you.

I believe $include does too, but I haven't tried

[jakemac, 2015-02-10 14:56:43]

New patchsets have been uploaded after l-g-t-m from sigmund@google.com

[jakemac, 2015-02-10 14:56:56]

https://codereview.chromium.org/906413002/diff/60001/pubspec.yaml
File pubspec.yaml (right):

https://codereview.chromium.org/906413002/diff/60001/pubspec.yaml#newcode27
pubspec.yaml:27: - test/init_method_test.dart
On 2015/02/10 01:45:48, Siggi Cherem (dart-lang) wrote:
> On 2015/02/09 23:41:05, jakemac wrote:
> > On 2015/02/09 22:55:17, Siggi Cherem (dart-lang) wrote:
> > > I believe you can also do:
> > > $include:
> > >  - test/initializer_test.dart
> > >  - ...
> > 
> > Hmm, should we do that for all transformers then? This is nice in that its
> > similar to the polymer and up-coming web_components transformers, but I
don't
> > have a strong opinion. 
> > 
> > Does $include support glob syntax?
> 
> it's mainly for iteration speed, so not entirely necessary. Up to you.
> 
> I believe $include does too, but I haven't tried

Done.

[jakemac, 2015-02-10 14:57:12]

Committed patchset #6 (id:100001) manually as
8ff3cf318c18345a5ac9b99e6c315313c11893f6 (presubmit successful).