Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(524)

Issue 1752393002: Add source mappings and inlined Dart code to diff_view. (Closed)

Created:
4 years, 9 months ago by Johnni Winther
Modified:
4 years, 9 months ago
CC:
reviews_dartlang.org
Base URL:
https://github.com/dart-lang/sdk.git@master
Target Ref:
refs/heads/master
Visibility:
Public.

Description

Add source mappings and inlined Dart code to diff_view. BUG= R=sigmund@google.com Committed: https://github.com/dart-lang/sdk/commit/72544512443d88c37bed3b9ad39094511dfa0a28

Patch Set 1 #

Total comments: 4

Patch Set 2 : Updated cf. comments. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1498 lines, -403 lines) Patch
M tests/compiler/dart2js/sourcemaps/diff.dart View 1 6 chunks +407 lines, -49 lines 0 comments Download
M tests/compiler/dart2js/sourcemaps/diff_view.dart View 1 17 chunks +595 lines, -165 lines 0 comments Download
M tests/compiler/dart2js/sourcemaps/html_parts.dart View 8 chunks +362 lines, -92 lines 0 comments Download
M tests/compiler/dart2js/sourcemaps/output_structure.dart View 10 chunks +87 lines, -16 lines 0 comments Download
M tests/compiler/dart2js/sourcemaps/sourcemap_helper.dart View 1 chunk +6 lines, -1 line 0 comments Download
M tests/compiler/dart2js/sourcemaps/sourcemap_html_helper.dart View 8 chunks +41 lines, -80 lines 0 comments Download

Messages

Total messages: 7 (2 generated)
Johnni Winther
4 years, 9 months ago (2016-03-02 15:14:52 UTC) #2
Johnni Winther
ping
4 years, 9 months ago (2016-03-07 09:00:36 UTC) #3
Siggi Cherem (dart-lang)
lgtm (very high level review, though) https://codereview.chromium.org/1752393002/diff/1/tests/compiler/dart2js/sourcemaps/diff.dart File tests/compiler/dart2js/sourcemaps/diff.dart (right): https://codereview.chromium.org/1752393002/diff/1/tests/compiler/dart2js/sourcemaps/diff.dart#newcode719 tests/compiler/dart2js/sourcemaps/diff.dart:719: static const String ...
4 years, 9 months ago (2016-03-07 18:08:36 UTC) #4
Johnni Winther
Committed patchset #2 (id:20001) manually as 72544512443d88c37bed3b9ad39094511dfa0a28 (presubmit successful).
4 years, 9 months ago (2016-03-08 09:02:36 UTC) #6
Johnni Winther
4 years, 9 months ago (2016-03-08 09:15:18 UTC) #7
Message was sent while issue was closed.
https://codereview.chromium.org/1752393002/diff/1/tests/compiler/dart2js/sour...
File tests/compiler/dart2js/sourcemaps/diff.dart (right):

https://codereview.chromium.org/1752393002/diff/1/tests/compiler/dart2js/sour...
tests/compiler/dart2js/sourcemaps/diff.dart:719: static const String mainDart =
'main_dart';
On 2016/03/07 18:08:36, Siggi Cherem (dart-lang) wrote:
> consider: mainDart => originalDart?
> 
> (mainDart sounds like it only applies to the main function)
> 
> What other categories of code do we want to have? For example, should we also
> have a `generated` category (typically anything that is expanded from keywords
> like async/await, defer loading, etc)

Done.

I don't have other categories in mind, but they tend to pop up along the way.

https://codereview.chromium.org/1752393002/diff/1/tests/compiler/dart2js/sour...
File tests/compiler/dart2js/sourcemaps/diff_view.dart (right):

https://codereview.chromium.org/1752393002/diff/1/tests/compiler/dart2js/sour...
tests/compiler/dart2js/sourcemaps/diff_view.dart:645: Map<Interval, CodeSource>
intervals = uriCodeSourceMap[codeSource.uri];
On 2016/03/07 18:08:36, Siggi Cherem (dart-lang) wrote:
> nti: format here and below

Done.

Powered by Google App Engine
This is Rietveld 408576698