Creating a common library for all DOM types.

Creating a common library for all DOM types.

BUG=6810
Committed: https://code.google.com/p/dart/source/detail?r=15564

[blois, 2012-11-20 18:57:52]

Not quite sure if this is the best name for the library, let me know if you have
some better suggestions.

[sra1, 2012-11-21 05:54:33]

lgtm

https://codereview.chromium.org/11316113/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/compiler.dart (right):

https://codereview.chromium.org/11316113/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/compiler.dart:501: || libraryName ==
'dart:html_common') {
Coding style is to put || at the end of the previous line.

http://www.dartlang.org/articles/style-guide/#do-place-the-operator-on-the-pr...

https://codereview.chromium.org/11316113/diff/1/sdk/lib/html/scripts/generato...
File sdk/lib/html/scripts/generator.py (right):

https://codereview.chromium.org/11316113/diff/1/sdk/lib/html/scripts/generato...
sdk/lib/html/scripts/generator.py:1085: 'FileList': TypeData(clazz='Interface',
item_type='File'),
Is this change necessary?

[blois, 2012-11-26 18:10:34]

https://codereview.chromium.org/11316113/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/compiler.dart (right):

https://codereview.chromium.org/11316113/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/compiler.dart:501: || libraryName ==
'dart:html_common') {
On 2012/11/21 05:54:33, sra1 wrote:
> Coding style is to put || at the end of the previous line.
> 
>
http://www.dartlang.org/articles/style-guide/#do-place-the-operator-on-the-pr...

Done.

https://codereview.chromium.org/11316113/diff/1/sdk/lib/html/scripts/generato...
File sdk/lib/html/scripts/generator.py (right):

https://codereview.chromium.org/11316113/diff/1/sdk/lib/html/scripts/generato...
sdk/lib/html/scripts/generator.py:1085: 'FileList': TypeData(clazz='Interface',
item_type='File'),
On 2012/11/21 05:54:33, sra1 wrote:
> Is this change necessary?

Conversions (for serializing the JS types) has moved into html_common, and needs
to access the FileList type, so we can't have it be private.

[sra1, 2012-11-26 18:14:57]

still lgtm

[ahe, 2012-11-27 07:57:05]

https://codereview.chromium.org/11316113/diff/3003/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/compiler.dart (left):

https://codereview.chromium.org/11316113/diff/3003/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/compiler.dart:491: || libraryName ==
'dart:mirrors'
Why are you changing the style here?

[blois, 2012-11-27 17:18:08]

https://codereview.chromium.org/11316113/diff/3003/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/compiler.dart (left):

https://codereview.chromium.org/11316113/diff/3003/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/compiler.dart:491: || libraryName ==
'dart:mirrors'
On 2012/11/27 07:57:05, ahe wrote:
> Why are you changing the style here?

Per previous feedback on this CL, the style guide states that operators should
be on the preceding line-

http://www.dartlang.org/articles/style-guide/#do-place-the-operator-on-the-pr...

[ahe, 2012-11-27 18:38:11]

On 2012/11/27 17:18:08, blois wrote:
>
https://codereview.chromium.org/11316113/diff/3003/sdk/lib/_internal/compiler...
> File sdk/lib/_internal/compiler/implementation/compiler.dart (left):
> 
>
https://codereview.chromium.org/11316113/diff/3003/sdk/lib/_internal/compiler...
> sdk/lib/_internal/compiler/implementation/compiler.dart:491: || libraryName ==
> 'dart:mirrors'
> On 2012/11/27 07:57:05, ahe wrote:
> > Why are you changing the style here?
> 
> Per previous feedback on this CL, the style guide states that operators should
> be on the preceding line-
> 
>
http://www.dartlang.org/articles/style-guide/#do-place-the-operator-on-the-pr...

Please follow the existing style.

[sra1, 2012-11-27 21:46:54]

https://codereview.chromium.org/11316113/diff/3003/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/compiler.dart (left):

https://codereview.chromium.org/11316113/diff/3003/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/compiler.dart:491: || libraryName ==
'dart:mirrors'
On 2012/11/27 17:18:08, blois wrote:
> On 2012/11/27 07:57:05, ahe wrote:
> > Why are you changing the style here?
> 
> Per previous feedback on this CL, the style guide states that operators should
> be on the preceding line-
> 
>
http://www.dartlang.org/articles/style-guide/#do-place-the-operator-on-the-pr...

Pete *is* following the existing style at line 610.
Since we have a style guide, we should follow the style guide where there is
inconsistency in the file being edited.

Within dart2js as a whole, the code follows the style guide at a ratio of about
3:1.  The rest should be brought into line with the style guide as we touch the
code.

[blois, 2012-11-27 22:37:25]

On 2012/11/27 21:46:54, sra1 wrote:
>
https://codereview.chromium.org/11316113/diff/3003/sdk/lib/_internal/compiler...
> File sdk/lib/_internal/compiler/implementation/compiler.dart (left):
> 
>
https://codereview.chromium.org/11316113/diff/3003/sdk/lib/_internal/compiler...
> sdk/lib/_internal/compiler/implementation/compiler.dart:491: || libraryName ==
> 'dart:mirrors'
> On 2012/11/27 17:18:08, blois wrote:
> > On 2012/11/27 07:57:05, ahe wrote:
> > > Why are you changing the style here?
> > 
> > Per previous feedback on this CL, the style guide states that operators
should
> > be on the preceding line-
> > 
> >
>
http://www.dartlang.org/articles/style-guide/#do-place-the-operator-on-the-pr...
> 
> Pete *is* following the existing style at line 610.
> Since we have a style guide, we should follow the style guide where there is
> inconsistency in the file being edited.
> 
> Within dart2js as a whole, the code follows the style guide at a ratio of
about
> 3:1.  The rest should be brought into line with the style guide as we touch
the
> code.

Correct, my understanding is that code which does not follow the style guide
should be cleaned up as it is updated. Given that this section is also
inconsistent with the style of the rest of the file, I believe that this is the
correct change.

[ahe, 2012-11-28 01:57:12]

On 2012/11/27 22:37:25, blois wrote:
> On 2012/11/27 21:46:54, sra1 wrote:
> >
>
https://codereview.chromium.org/11316113/diff/3003/sdk/lib/_internal/compiler...
> > File sdk/lib/_internal/compiler/implementation/compiler.dart (left):
> > 
> >
>
https://codereview.chromium.org/11316113/diff/3003/sdk/lib/_internal/compiler...
> > sdk/lib/_internal/compiler/implementation/compiler.dart:491: || libraryName
==
> > 'dart:mirrors'
> > On 2012/11/27 17:18:08, blois wrote:
> > > On 2012/11/27 07:57:05, ahe wrote:
> > > > Why are you changing the style here?
> > > 
> > > Per previous feedback on this CL, the style guide states that operators
> should
> > > be on the preceding line-
> > > 
> > >
> >
>
http://www.dartlang.org/articles/style-guide/#do-place-the-operator-on-the-pr...
> > 
> > Pete *is* following the existing style at line 610.
> > Since we have a style guide, we should follow the style guide where there is
> > inconsistency in the file being edited.
> > 
> > Within dart2js as a whole, the code follows the style guide at a ratio of
> about
> > 3:1.  The rest should be brought into line with the style guide as we touch
> the
> > code.
> 
> Correct, my understanding is that code which does not follow the style guide
> should be cleaned up as it is updated. Given that this section is also
> inconsistent with the style of the rest of the file, I believe that this is
the
> correct change.

Please stick to minimal changes.  This makes it easier for us to figure out why
something was changed.

The dart2js team (this includes Stephen) can discuss changing our practices, but
I don't think this CL should be tied up in that discussion.

[blois, 2012-11-29 17:32:42]

On 2012/11/28 01:57:12, ahe wrote:
> On 2012/11/27 22:37:25, blois wrote:
> > On 2012/11/27 21:46:54, sra1 wrote:
> > >
> >
>
https://codereview.chromium.org/11316113/diff/3003/sdk/lib/_internal/compiler...
> > > File sdk/lib/_internal/compiler/implementation/compiler.dart (left):
> > > 
> > >
> >
>
https://codereview.chromium.org/11316113/diff/3003/sdk/lib/_internal/compiler...
> > > sdk/lib/_internal/compiler/implementation/compiler.dart:491: ||
libraryName
> ==
> > > 'dart:mirrors'
> > > On 2012/11/27 17:18:08, blois wrote:
> > > > On 2012/11/27 07:57:05, ahe wrote:
> > > > > Why are you changing the style here?
> > > > 
> > > > Per previous feedback on this CL, the style guide states that operators
> > should
> > > > be on the preceding line-
> > > > 
> > > >
> > >
> >
>
http://www.dartlang.org/articles/style-guide/#do-place-the-operator-on-the-pr...
> > > 
> > > Pete *is* following the existing style at line 610.
> > > Since we have a style guide, we should follow the style guide where there
is
> > > inconsistency in the file being edited.
> > > 
> > > Within dart2js as a whole, the code follows the style guide at a ratio of
> > about
> > > 3:1.  The rest should be brought into line with the style guide as we
touch
> > the
> > > code.
> > 
> > Correct, my understanding is that code which does not follow the style guide
> > should be cleaned up as it is updated. Given that this section is also
> > inconsistent with the style of the rest of the file, I believe that this is
> the
> > correct change.
> 
> Please stick to minimal changes.  This makes it easier for us to figure out
why
> something was changed.
> 
> The dart2js team (this includes Stephen) can discuss changing our practices,
but
> I don't think this CL should be tied up in that discussion.

The formatting change has been reverted, I'd like to get this in ASAP. PTAL.

[ahe, 2012-11-29 17:39:24]

Changes to sdk/lib/_internal/compiler/, LGTM. Are you sure about adding
FileList?  I thought Emily removed that class.

[blois, 2012-11-29 17:43:50]

On 2012/11/29 17:39:24, ahe wrote:
> Changes to sdk/lib/_internal/compiler/, LGTM. Are you sure about adding
> FileList?  I thought Emily removed that class.


(Stephen asked about it earlier)- this changes FileList from private to public,
which is needed since it has to be referenced from html_common.