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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 10384142: Initial support for 'Extract temp' - only when single expression selected (Closed)

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

Description

Initial support for 'Extract temp' - only when single expression selected Actually, already can be useful. For example to extract single argument of invocation into variable. R=messick@google.com BUG= TEST= Committed: https://code.google.com/p/dart/source/detail?r=7590

Patch Set 1 #

Total comments: 17

Patch Set 2 : Changes after review comments #

Patch Set 3 : Use ASTVisitor as type switch #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1354 lines, -139 lines) Patch
M compiler/java/com/google/dart/compiler/ast/ASTVisitor.java View 1 2 chunks +6 lines, -2 lines 0 comments Download
M editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/internal/dom/rewrite/ASTRewriteAnalyzer.java View 1 chunk +7 lines, -1 line 0 comments Download
M editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/internal/dom/rewrite/RewriteEventStore.java View 2 chunks +2 lines, -3 lines 0 comments Download
M editor/tools/plugins/com.google.dart.tools.ui/META-INF/MANIFEST.MF View 2 chunks +3 lines, -1 line 0 comments Download
M editor/tools/plugins/com.google.dart.tools.ui/plugin.xml View 1 4 chunks +21 lines, -5 lines 0 comments Download
M editor/tools/plugins/com.google.dart.tools.ui/src/com/google/dart/tools/internal/corext/refactoring/Checks.java View 5 chunks +60 lines, -49 lines 0 comments Download
M editor/tools/plugins/com.google.dart.tools.ui/src/com/google/dart/tools/internal/corext/refactoring/RefactoringAvailabilityTester.java View 1 3 chunks +12 lines, -6 lines 0 comments Download
M editor/tools/plugins/com.google.dart.tools.ui/src/com/google/dart/tools/internal/corext/refactoring/RefactoringCoreMessages.java View 1 4 chunks +9 lines, -10 lines 0 comments Download
M editor/tools/plugins/com.google.dart.tools.ui/src/com/google/dart/tools/internal/corext/refactoring/RefactoringCoreMessages.properties View 1 3 chunks +9 lines, -5 lines 0 comments Download
A editor/tools/plugins/com.google.dart.tools.ui/src/com/google/dart/tools/internal/corext/refactoring/code/ExtractLocalRefactoring.java View 1 1 chunk +232 lines, -0 lines 0 comments Download
A editor/tools/plugins/com.google.dart.tools.ui/src/com/google/dart/tools/internal/corext/refactoring/code/ExtractUtils.java View 1 2 1 chunk +269 lines, -0 lines 0 comments Download
A editor/tools/plugins/com.google.dart.tools.ui/src/com/google/dart/tools/ui/actions/ExtractLocalAction.java View 1 1 chunk +59 lines, -0 lines 0 comments Download
M editor/tools/plugins/com.google.dart.tools.ui/src/com/google/dart/tools/ui/actions/JdtActionConstants.java View 1 2 chunks +11 lines, -13 lines 0 comments Download
M editor/tools/plugins/com.google.dart.tools.ui/src/com/google/dart/tools/ui/actions/RefactorActionGroup.java View 1 6 chunks +14 lines, -8 lines 0 comments Download
A editor/tools/plugins/com.google.dart.tools.ui/src/com/google/dart/tools/ui/internal/refactoring/ExtractLocalWizard.java View 1 1 chunk +148 lines, -0 lines 0 comments Download
M editor/tools/plugins/com.google.dart.tools.ui/src/com/google/dart/tools/ui/internal/refactoring/RefactoringMessages.java View 1 2 chunks +10 lines, -12 lines 0 comments Download
M editor/tools/plugins/com.google.dart.tools.ui/src/com/google/dart/tools/ui/internal/refactoring/RefactoringMessages.properties View 1 3 chunks +12 lines, -7 lines 0 comments Download
M editor/tools/plugins/com.google.dart.tools.ui/src/com/google/dart/tools/ui/internal/text/DartHelpContextIds.java View 1 2 chunks +2 lines, -2 lines 0 comments Download
M editor/tools/plugins/com.google.dart.tools.ui/src/com/google/dart/tools/ui/internal/text/Selection.java View 4 chunks +11 lines, -11 lines 0 comments Download
M editor/tools/plugins/com.google.dart.tools.ui_test/META-INF/MANIFEST.MF View 1 chunk +4 lines, -3 lines 0 comments Download
M editor/tools/plugins/com.google.dart.tools.ui_test/src/com/google/dart/tools/ui/refactoring/AbstractDartTest.java View 1 chunk +0 lines, -1 line 0 comments Download
A editor/tools/plugins/com.google.dart.tools.ui_test/src/com/google/dart/tools/ui/refactoring/ExtractLocalRefactoringTest.java View 1 1 chunk +142 lines, -0 lines 0 comments Download
A editor/tools/plugins/com.google.dart.tools.ui_test/src/com/google/dart/tools/ui/refactoring/ExtractUtilsTest.java View 1 2 1 chunk +306 lines, -0 lines 0 comments Download
M editor/tools/plugins/com.google.dart.tools.ui_test/src/com/google/dart/tools/ui/refactoring/TestAll.java View 1 2 chunks +5 lines, -0 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
scheglov
8 years, 7 months ago (2012-05-12 05:22:12 UTC) #1
messick
Looking forward to using this! Where does "change method signature" fall in the priority list? ...
8 years, 7 months ago (2012-05-12 15:32:53 UTC) #2
scheglov
8 years, 7 months ago (2012-05-13 05:05:41 UTC) #3 
https://chromiumcodereview.appspot.com/10384142/diff/1/editor/tools/plugins/c...
File
editor/tools/plugins/com.google.dart.tools.ui/src/com/google/dart/tools/internal/corext/refactoring/code/ExtractTempRefactoring.java
(right):

https://chromiumcodereview.appspot.com/10384142/diff/1/editor/tools/plugins/c...
editor/tools/plugins/com.google.dart.tools.ui/src/com/google/dart/tools/internal/corext/refactoring/code/ExtractTempRefactoring.java:49:
public class ExtractTempRefactoring extends Refactoring {
On 2012/05/12 15:32:53, messick wrote:
> ExtractLocalRefactoring would be a better name.
> If you decide to change the class name then method (public at least) names
that
> include "Temp" should also be changed.

Done.

https://chromiumcodereview.appspot.com/10384142/diff/1/editor/tools/plugins/c...
editor/tools/plugins/com.google.dart.tools.ui/src/com/google/dart/tools/internal/corext/refactoring/code/ExtractTempRefactoring.java:61:
private String[] fGuessedTempNames;
On 2012/05/12 15:32:53, messick wrote:
> Inconsistent name.

Done.

https://chromiumcodereview.appspot.com/10384142/diff/1/editor/tools/plugins/c...
File
editor/tools/plugins/com.google.dart.tools.ui/src/com/google/dart/tools/internal/corext/refactoring/code/ExtractUtils.java
(right):

https://chromiumcodereview.appspot.com/10384142/diff/1/editor/tools/plugins/c...
editor/tools/plugins/com.google.dart.tools.ui/src/com/google/dart/tools/internal/corext/refactoring/code/ExtractUtils.java:50:
public static final String DEFAULT_END_OF_LINE =
System.getProperty("line.separator", "\n");
On 2012/05/12 15:32:53, messick wrote:
> In practice, this is probably fine. The value is inconsistent with the comment
> describing the constant, though.

I can not understand why value i inconsistent with comment, can you clarify
please?

https://chromiumcodereview.appspot.com/10384142/diff/1/editor/tools/plugins/c...
editor/tools/plugins/com.google.dart.tools.ui/src/com/google/dart/tools/internal/corext/refactoring/code/ExtractUtils.java:73:
public static String getNodePrefix(Buffer buffer, DartNode node) {
On 2012/05/12 15:32:53, messick wrote:
> This will not work with some bizarre formatting options.
> See DartIndenter.computeIndentation()
> (Not sure that is usable here, but you should check it out if you have not
done
> so already.)

In general I don't like to use something more complex until I see example where
this is really required.
I will consider using DartIndenter on future steps, probably will wrap Buffer
into some IDocument to get EOL, indentation, etc.

https://chromiumcodereview.appspot.com/10384142/diff/1/editor/tools/plugins/c...
editor/tools/plugins/com.google.dart.tools.ui/src/com/google/dart/tools/internal/corext/refactoring/code/ExtractUtils.java:99:
public static String getTypeSource(DartExpression expression) {
On 2012/05/12 15:32:53, messick wrote:
> Once again, I'm surprised you chose not to use a visitor. :)

Done.

https://chromiumcodereview.appspot.com/10384142/diff/1/editor/tools/plugins/c...
File
editor/tools/plugins/com.google.dart.tools.ui/src/com/google/dart/tools/internal/corext/refactoring/structure/CompilationUnitRewrite.java
(right):

https://chromiumcodereview.appspot.com/10384142/diff/1/editor/tools/plugins/c...
editor/tools/plugins/com.google.dart.tools.ui/src/com/google/dart/tools/internal/corext/refactoring/structure/CompilationUnitRewrite.java:33:
* . Statements recovery is enabled by default, but can be disabled with
On 2012/05/12 15:32:53, messick wrote:
> Fill paragraph (and delete space before dot)

Actually I don't use this class, I will roll it back.

https://chromiumcodereview.appspot.com/10384142/diff/1/editor/tools/plugins/c...
File
editor/tools/plugins/com.google.dart.tools.ui/src/com/google/dart/tools/ui/actions/ExtractTempAction.java
(right):

https://chromiumcodereview.appspot.com/10384142/diff/1/editor/tools/plugins/c...
editor/tools/plugins/com.google.dart.tools.ui/src/com/google/dart/tools/ui/actions/ExtractTempAction.java:22:
public class ExtractTempAction extends SelectionDispatchAction {
On 2012/05/12 15:32:53, messick wrote:
> ExtractLocalAction would be a better name.

Done.

https://chromiumcodereview.appspot.com/10384142/diff/1/editor/tools/plugins/c...
File
editor/tools/plugins/com.google.dart.tools.ui/src/com/google/dart/tools/ui/internal/refactoring/ExtractTempWizard.java
(right):

https://chromiumcodereview.appspot.com/10384142/diff/1/editor/tools/plugins/c...
editor/tools/plugins/com.google.dart.tools.ui/src/com/google/dart/tools/ui/internal/refactoring/ExtractTempWizard.java:23:
public class ExtractTempWizard extends RefactoringWizard {
On 2012/05/12 15:32:53, messick wrote:
> ExtractLocalWizard would be a better name.

Done.

Powered by Google App Engine
This is Rietveld 408576698