Merge pull request #3233 from TomMalbran/tom/fix-issue-3130

jasonsanjose · jasonsanjose · commit e135caf08bcd · 2013-04-16T17:58:34.000-07:00
Fix #3130: Move line up/down has incorrect behavior in edge cases in inline editor
diff --git a/src/editor/EditorCommandHandlers.js b/src/editor/EditorCommandHandlers.js
@@ -625,11 +625,13 @@ define(function (require, exports, module) {
         
         var doc = editor.document,
             sel = editor.getSelection(),
-            originalSel  = editor.getSelection(),
-            hasSelection = (sel.start.line !== sel.end.line) || (sel.start.ch !== sel.end.ch),
-            inlineWidget = EditorManager.getFocusedInlineWidget(),
-            firstLine    = editor.getFirstVisibleLine(),
-            lastLine     = editor.getLastVisibleLine();
+            originalSel    = editor.getSelection(),
+            hasSelection   = (sel.start.line !== sel.end.line) || (sel.start.ch !== sel.end.ch),
+            isInlineWidget = !!EditorManager.getFocusedInlineWidget(),
+            firstLine      = editor.getFirstVisibleLine(),
+            lastLine       = editor.getLastVisibleLine(),
+            totalLines     = editor.lineCount(),
+            lineLength     = 0;
         
         sel.start.ch = 0;
         // The end of the selection becomes the start of the next line, if it isn't already
@@ -645,7 +647,13 @@ define(function (require, exports, module) {
                     var prevText = doc.getRange({ line: sel.start.line - 1, ch: 0 }, sel.start);
                     
                     if (sel.end.line === lastLine + 1) {
-                        prevText = "\n" + prevText.substring(0, prevText.length - 1);
+                        if (isInlineWidget) {
+                            prevText   = prevText.substring(0, prevText.length - 1);
+                            lineLength = doc.getLine(sel.end.line - 1).length;
+                            doc.replaceRange("\n", { line: sel.end.line - 1, ch: lineLength });
+                        } else {
+                            prevText = "\n" + prevText.substring(0, prevText.length - 1);
+                        }
                     }
                     
                     doc.replaceRange("", { line: sel.start.line - 1, ch: 0 }, sel.start);
@@ -663,17 +671,28 @@ define(function (require, exports, module) {
             }
             break;
         case DIRECTION_DOWN:
-            if (sel.end.line <= lastLine + (inlineWidget ? -1 : 1)) {
+            if (sel.end.line <= lastLine) {
                 doc.batchOperation(function () {
-                    var nextText = doc.getRange(sel.end, { line: sel.end.line + 1, ch: 0 });
+                    var nextText      = doc.getRange(sel.end, { line: sel.end.line + 1, ch: 0 }),
+                        deletionStart = sel.end;
                     
-                    var deletionStart = sel.end;
-                    if (!inlineWidget && sel.end.line === lastLine) {
-                        nextText += "\n";
-                        deletionStart = { line: sel.end.line - 1, ch: doc.getLine(sel.end.line - 1).length };
+                    if (sel.end.line === lastLine) {
+                        if (isInlineWidget) {
+                            if (sel.end.line === totalLines - 1) {
+                                nextText += "\n";
+                            }
+                            lineLength = doc.getLine(sel.end.line - 1).length;
+                            doc.replaceRange("\n", { line: sel.end.line, ch: doc.getLine(sel.end.line).length });
+                        } else {
+                            nextText     += "\n";
+                            deletionStart = { line: sel.end.line - 1, ch: doc.getLine(sel.end.line - 1).length };
+                        }
                     }
     
                     doc.replaceRange("", deletionStart, { line: sel.end.line + 1, ch: 0 });
+                    if (lineLength) {
+                        doc.replaceRange("", { line: sel.end.line - 1, ch: lineLength }, { line: sel.end.line, ch: 0 });
+                    }
                     doc.replaceRange(nextText, { line: sel.start.line, ch: 0 });
                 });
             }
diff --git a/test/spec/EditorCommandHandlers-test-files/test.css b/test/spec/EditorCommandHandlers-test-files/test.css
@@ -0,0 +1,3 @@
+.testClass {
+    color: red;
+}
diff --git a/test/spec/EditorCommandHandlers-test-files/test.html b/test/spec/EditorCommandHandlers-test-files/test.html
@@ -0,0 +1,11 @@
+<!doctype html>
+<html>
+<head>
+<title>Simple Test</title>
+<link rel="stylesheet" href="test.css">
+</head>
+
+<body>
+<p class="testClass">Brackets is awesome!</p>
+</body>
+</html>
diff --git a/test/spec/EditorCommandHandlers-test.js b/test/spec/EditorCommandHandlers-test.js
@@ -22,12 +22,13 @@
  */
 
 /*jslint vars: true, plusplus: true, devel: true, nomen: true, indent: 4, maxerr: 50 */
-/*global define, describe, it, expect, beforeEach, afterEach, waitsFor, waits, runs, $ */
+/*global define, describe, it, expect, beforeEach, afterEach, waitsFor, waits, runs, $, waitsForDone */
 
 define(function (require, exports, module) {
     'use strict';
     
     var Editor                = require("editor/Editor").Editor,
+        EditorManager         = require("editor/EditorManager"),
         EditorCommandHandlers = require("editor/EditorCommandHandlers"),
         Commands              = require("command/Commands"),
         CommandManager        = require("command/CommandManager"),
@@ -59,19 +60,23 @@ define(function (require, exports, module) {
             myEditor.focus();
         }
         
-        function makeEditorWithRange(range) {
+        function makeEditorWithRange(range, content) {
+            content = content || defaultContent;
+            
             // create editor with a visible range
-            var mocks = SpecRunnerUtils.createMockEditor(defaultContent, "javascript", range);
+            var mocks = SpecRunnerUtils.createMockEditor(content, "javascript", range);
             myDocument = mocks.doc;
             myEditor = mocks.editor;
             
             myEditor.focus();
         }
         
         afterEach(function () {
-            SpecRunnerUtils.destroyMockEditor(myDocument);
-            myEditor = null;
-            myDocument = null;
+            if (myDocument) {
+                SpecRunnerUtils.destroyMockEditor(myDocument);
+                myEditor = null;
+                myDocument = null;
+            }
         });
         
         
@@ -2065,6 +2070,119 @@ define(function (require, exports, module) {
         });
         
         
+        describe("Move Lines Up/Down - inline editor", function () {
+            this.category = "integration";
+            
+            var testWindow, promise, editor;
+            var testPath = SpecRunnerUtils.getTestPath("/spec/EditorCommandHandlers-test-files");
+            
+            var moveContent = ".testClass {\n" +
+                              "    color: red;\n" +
+                              "}";
+            
+            beforeEach(function () {
+                if (!testWindow) {
+                    SpecRunnerUtils.createTestWindowAndRun(this, function (w) {
+                        testWindow = w;
+                        
+                        // Load module instances from brackets.test
+                        CommandManager      = testWindow.brackets.test.CommandManager;
+                        Commands            = testWindow.brackets.test.Commands;
+                        EditorManager       = testWindow.brackets.test.EditorManager;
+                       
+                        SpecRunnerUtils.loadProjectInTestWindow(testPath);
+                    });
+                }
+                
+                runs(function () {
+                    promise = CommandManager.execute(Commands.FILE_ADD_TO_WORKING_SET, {fullPath: testPath + "/test.html"});
+                    waitsForDone(promise, "Open into working set");
+                });
+                
+                runs(function () {
+                    // Open inline editor onto test.css's ".testClass" rule
+                    promise = SpecRunnerUtils.toggleQuickEditAtOffset(EditorManager.getCurrentFullEditor(), {line: 8, ch: 11});
+                    waitsForDone(promise, "Open inline editor");
+                });
+                
+                runs(function () {
+                    editor = EditorManager.getCurrentFullEditor().getInlineWidgets()[0].editors[0];
+                });
+            });
+            
+            afterEach(function () {
+                runs(function () {
+                    var promise = CommandManager.execute(Commands.FILE_CLOSE_ALL);
+                    waitsForDone(promise, "Close all open files in working set");
+                    
+                    // Close the save dialog without saving the changes
+                    var $dlg = testWindow.$(".modal.instance");
+                    if ($dlg.length) {
+                        SpecRunnerUtils.clickDialogButton("dontsave");
+                    }
+                });
+            });
+            
+            
+            it("should not move the first line of the inline editor up", function () {
+                editor.setCursorPos({line: 0, ch: 5});
+                CommandManager.execute(Commands.EDIT_LINE_UP, editor);
+                
+                expect(editor.document.getText()).toEqual(moveContent);
+                expect(editor._codeMirror.doc.historySize().undo).toBe(0);
+                expect(editor.getFirstVisibleLine()).toBe(0);
+                expect(editor.getLastVisibleLine()).toBe(2);
+            });
+            
+            it("should not move the last line of the inline editor down", function () {
+                editor.setCursorPos({line: 2, ch: 5});
+                CommandManager.execute(Commands.EDIT_LINE_DOWN, editor);
+                
+                expect(editor.document.getText()).toEqual(moveContent);
+                expect(editor._codeMirror.doc.historySize().undo).toBe(0);
+                expect(editor.getFirstVisibleLine()).toBe(0);
+                expect(editor.getLastVisibleLine()).toBe(2);
+            });
+            
+            it("should be able to move the second to last line of the inline editor down", function () {
+                editor.setCursorPos({line: 1, ch: 5});
+                CommandManager.execute(Commands.EDIT_LINE_DOWN, editor);
+                
+                var lines = moveContent.split("\n");
+                var temp = lines[1];
+                lines[1] = lines[2];
+                lines[2] = temp;
+                var expectedText = lines.join("\n");
+                
+                expect(editor.document.getText()).toEqual(expectedText);
+                expect(editor.getFirstVisibleLine()).toBe(0);
+                expect(editor.getLastVisibleLine()).toBe(2);
+            });
+            
+            it("should be able to move the last line of the inline editor up", function () {
+                editor.setCursorPos({line: 2, ch: 0});
+                CommandManager.execute(Commands.EDIT_LINE_UP, editor);
+                
+                var lines = moveContent.split("\n");
+                var temp = lines[1];
+                lines[1] = lines[2];
+                lines[2] = temp;
+                var expectedText = lines.join("\n");
+                
+                expect(editor.document.getText()).toEqual(expectedText);
+                expect(editor.getFirstVisibleLine()).toBe(0);
+                expect(editor.getLastVisibleLine()).toBe(2);
+                
+                // This must be in the last spec in the suite.
+                runs(function () {
+                    this.after(function () {
+                        SpecRunnerUtils.closeTestWindow();
+                    });
+                });
+            });
+        });
+        
+        
         describe("Delete Line", function () {
             beforeEach(setupFullEditor);
             

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+.testClass {`
	`2`	`+ color: red;`
	`3`	`+}`