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

Issue 10656034: Show override indicator. (Closed)

Created:
8 years, 6 months ago by scheglov
Modified:
8 years, 6 months ago
CC:
reviews_dartlang.org
Visibility:
Public.

Description

Patch Set 1 #

Total comments: 12
Unified diffs Side-by-side diffs Delta from patch set Stats (+183 lines, -151 lines) Patch
M compiler/java/com/google/dart/compiler/resolver/DynamicElementImplementation.java View 3 chunks +7 lines, -0 lines 0 comments Download
M compiler/java/com/google/dart/compiler/resolver/Elements.java View 1 chunk +4 lines, -1 line 0 comments Download
M compiler/java/com/google/dart/compiler/resolver/MethodElement.java View 2 chunks +6 lines, -0 lines 2 comments Download
M compiler/java/com/google/dart/compiler/resolver/MethodElementImplementation.java View 4 chunks +11 lines, -0 lines 2 comments Download
M compiler/java/com/google/dart/compiler/resolver/SyntheticDefaultConstructorElement.java View 3 chunks +7 lines, -0 lines 2 comments Download
M compiler/java/com/google/dart/compiler/type/TypeAnalyzer.java View 2 chunks +2 lines, -0 lines 0 comments Download
M editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/utilities/ast/DartElementLocator.java View 2 chunks +10 lines, -2 lines 2 comments Download
M editor/tools/plugins/com.google.dart.tools.ui/plugin.xml View 1 chunk +21 lines, -0 lines 4 comments Download
M editor/tools/plugins/com.google.dart.tools.ui/src/com/google/dart/tools/ui/internal/text/editor/OverrideIndicatorImageProvider.java View 4 chunks +6 lines, -17 lines 0 comments Download
M editor/tools/plugins/com.google.dart.tools.ui/src/com/google/dart/tools/ui/internal/text/editor/OverrideIndicatorManager.java View 5 chunks +109 lines, -131 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
scheglov
8 years, 6 months ago (2012-06-26 03:45:39 UTC) #1
pquitslund
LGTM. Aside from some superficial nits, this is awesome! https://chromiumcodereview.appspot.com/10656034/diff/1/compiler/java/com/google/dart/compiler/resolver/MethodElement.java File compiler/java/com/google/dart/compiler/resolver/MethodElement.java (right): https://chromiumcodereview.appspot.com/10656034/diff/1/compiler/java/com/google/dart/compiler/resolver/MethodElement.java#newcode1 compiler/java/com/google/dart/compiler/resolver/MethodElement.java:1: ...
8 years, 6 months ago (2012-06-26 04:03:01 UTC) #2
Brian Wilkerson
LGTM https://chromiumcodereview.appspot.com/10656034/diff/1/editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/utilities/ast/DartElementLocator.java File editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/utilities/ast/DartElementLocator.java (right): https://chromiumcodereview.appspot.com/10656034/diff/1/editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/utilities/ast/DartElementLocator.java#newcode112 editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/utilities/ast/DartElementLocator.java:112: * XXX Were you planning on writing a ...
8 years, 6 months ago (2012-06-26 14:06:20 UTC) #3
scheglov
8 years, 6 months ago (2012-06-26 14:35:12 UTC) #4
https://chromiumcodereview.appspot.com/10656034/diff/1/compiler/java/com/goog...
File compiler/java/com/google/dart/compiler/resolver/MethodElement.java (right):

https://chromiumcodereview.appspot.com/10656034/diff/1/compiler/java/com/goog...
compiler/java/com/google/dart/compiler/resolver/MethodElement.java:1: //
Copyright (c) 2011, the Dart project authors.  Please see the AUTHORS file
On 2012/06/26 04:03:01, pquitslund wrote:
> Nit: copyright => 2012

Done.

https://chromiumcodereview.appspot.com/10656034/diff/1/compiler/java/com/goog...
File
compiler/java/com/google/dart/compiler/resolver/MethodElementImplementation.java
(right):

https://chromiumcodereview.appspot.com/10656034/diff/1/compiler/java/com/goog...
compiler/java/com/google/dart/compiler/resolver/MethodElementImplementation.java:1:
// Copyright (c) 2011, the Dart project authors.  Please see the AUTHORS file
On 2012/06/26 04:03:01, pquitslund wrote:
> Nit: copyright => 2012

Done.

https://chromiumcodereview.appspot.com/10656034/diff/1/compiler/java/com/goog...
File
compiler/java/com/google/dart/compiler/resolver/SyntheticDefaultConstructorElement.java
(right):

https://chromiumcodereview.appspot.com/10656034/diff/1/compiler/java/com/goog...
compiler/java/com/google/dart/compiler/resolver/SyntheticDefaultConstructorElement.java:1:
// Copyright (c) 2011, the Dart project authors.  Please see the AUTHORS file
On 2012/06/26 04:03:01, pquitslund wrote:
> Nit: copyright => 2012

Done.

https://chromiumcodereview.appspot.com/10656034/diff/1/editor/tools/plugins/c...
File
editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/utilities/ast/DartElementLocator.java
(right):

https://chromiumcodereview.appspot.com/10656034/diff/1/editor/tools/plugins/c...
editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/utilities/ast/DartElementLocator.java:112:
* XXX
On 2012/06/26 14:06:20, Brian Wilkerson wrote:
> Were you planning on writing a comment here?

Done.

https://chromiumcodereview.appspot.com/10656034/diff/1/editor/tools/plugins/c...
File editor/tools/plugins/com.google.dart.tools.ui/plugin.xml (right):

https://chromiumcodereview.appspot.com/10656034/diff/1/editor/tools/plugins/c...
editor/tools/plugins/com.google.dart.tools.ui/plugin.xml:488: </action>-->
On 2012/06/26 04:03:01, pquitslund wrote:
> Can we remove this (albeit commented out) JDT reference?

From the rest of this file it looks for me that our approach is to have
everything, but commented and uncomment things once we are ready to implement
them.

https://chromiumcodereview.appspot.com/10656034/diff/1/editor/tools/plugins/c...
editor/tools/plugins/com.google.dart.tools.ui/plugin.xml:490:
label="%JavaSelectRulerAction.label"
On 2012/06/26 04:03:01, pquitslund wrote:
> %DartSelectRulerAction.label?

Done.

Powered by Google App Engine
This is Rietveld 408576698