From f96be45fa06ed20582baccc3314d030c982393fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Malbr=C3=A1n?= Date: Sun, 24 Mar 2013 04:37:58 -0300 Subject: [PATCH 1/4] Adding special cases when moving lines up or down at the last lines of an inline editor --- src/editor/EditorCommandHandlers.js | 35 ++++++++++++++++++++++------- 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/src/editor/EditorCommandHandlers.js b/src/editor/EditorCommandHandlers.js index 316e3f5ba3c..6f9383654dd 100644 --- a/src/editor/EditorCommandHandlers.js +++ b/src/editor/EditorCommandHandlers.js @@ -629,7 +629,9 @@ define(function (require, exports, module) { hasSelection = (sel.start.line !== sel.end.line) || (sel.start.ch !== sel.end.ch), inlineWidget = EditorManager.getFocusedInlineWidget(), firstLine = editor.getFirstVisibleLine(), - lastLine = editor.getLastVisibleLine(); + 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 (inlineWidget) { + 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 (inlineWidget) { + 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 }); }); } From e2ba2210a9c7bf1e46c0bea30f2453cec7e681a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Malbr=C3=A1n?= Date: Tue, 26 Mar 2013 18:48:56 -0300 Subject: [PATCH 2/4] Tests for the new special cases in inline editors --- src/editor/EditorCommandHandlers.js | 18 +-- .../EditorCommandHandlers-test-files/test.css | 3 + .../test.html | 11 ++ test/spec/EditorCommandHandlers-test.js | 115 +++++++++++++++++- 4 files changed, 132 insertions(+), 15 deletions(-) create mode 100644 test/spec/EditorCommandHandlers-test-files/test.css create mode 100644 test/spec/EditorCommandHandlers-test-files/test.html diff --git a/src/editor/EditorCommandHandlers.js b/src/editor/EditorCommandHandlers.js index 6f9383654dd..7280abcc0ac 100644 --- a/src/editor/EditorCommandHandlers.js +++ b/src/editor/EditorCommandHandlers.js @@ -625,13 +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(), - totalLines = editor.lineCount(), - lineLength = 0; + 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 @@ -647,7 +647,7 @@ define(function (require, exports, module) { var prevText = doc.getRange({ line: sel.start.line - 1, ch: 0 }, sel.start); if (sel.end.line === lastLine + 1) { - if (inlineWidget) { + 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 }); @@ -677,7 +677,7 @@ define(function (require, exports, module) { deletionStart = sel.end; if (sel.end.line === lastLine) { - if (inlineWidget) { + if (isInlineWidget) { if (sel.end.line === totalLines - 1) { nextText += "\n"; } diff --git a/test/spec/EditorCommandHandlers-test-files/test.css b/test/spec/EditorCommandHandlers-test-files/test.css new file mode 100644 index 00000000000..47778cacff5 --- /dev/null +++ b/test/spec/EditorCommandHandlers-test-files/test.css @@ -0,0 +1,3 @@ +.testClass { + color: red; +} \ No newline at end of file diff --git a/test/spec/EditorCommandHandlers-test-files/test.html b/test/spec/EditorCommandHandlers-test-files/test.html new file mode 100644 index 00000000000..74234aaf331 --- /dev/null +++ b/test/spec/EditorCommandHandlers-test-files/test.html @@ -0,0 +1,11 @@ + + + +Simple Test + + + + +

Brackets is awesome!

+ + \ No newline at end of file diff --git a/test/spec/EditorCommandHandlers-test.js b/test/spec/EditorCommandHandlers-test.js index c0e5f4b5b80..7684aeb4fdc 100644 --- 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,9 +60,11 @@ 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; @@ -69,9 +72,11 @@ define(function (require, exports, module) { } afterEach(function () { - SpecRunnerUtils.destroyMockEditor(myDocument); - myEditor = null; - myDocument = null; + if (myDocument) { + SpecRunnerUtils.destroyMockEditor(myDocument); + myEditor = null; + myDocument = null; + } }); @@ -2065,6 +2070,104 @@ define(function (require, exports, module) { }); + describe("Move Lines Up/Down - inline editor", function () { + 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]; + }); + } + }); + + 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.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.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); + + // Restore the content + editor.document.setText(moveContent); + }); + + 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); From ac079bc8de0c8de2284f76eafe9290a1059fdd62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Malbr=C3=A1n?= Date: Tue, 9 Apr 2013 03:51:08 -0300 Subject: [PATCH 3/4] Tests fixes after first review --- test/spec/EditorCommandHandlers-test.js | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/test/spec/EditorCommandHandlers-test.js b/test/spec/EditorCommandHandlers-test.js index 7684aeb4fdc..022081abd57 100644 --- a/test/spec/EditorCommandHandlers-test.js +++ b/test/spec/EditorCommandHandlers-test.js @@ -2071,6 +2071,8 @@ 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"); @@ -2108,11 +2110,18 @@ define(function (require, exports, module) { } }); + afterEach(function () { + // Restore the content + editor.document.setText(moveContent); + }); + + 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); }); @@ -2122,6 +2131,7 @@ define(function (require, exports, module) { CommandManager.execute(Commands.EDIT_LINE_DOWN, editor); expect(editor.document.getText()).toEqual(moveContent); + expect(editor._codeMirror.doc.historySize().undo).toBe(1); expect(editor.getFirstVisibleLine()).toBe(0); expect(editor.getLastVisibleLine()).toBe(2); }); @@ -2139,9 +2149,6 @@ define(function (require, exports, module) { expect(editor.document.getText()).toEqual(expectedText); expect(editor.getFirstVisibleLine()).toBe(0); expect(editor.getLastVisibleLine()).toBe(2); - - // Restore the content - editor.document.setText(moveContent); }); it("should be able to move the last line of the inline editor up", function () { From e59dcccfb49071161edd06ee5c5ad5c3067343dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Malbr=C3=A1n?= Date: Tue, 16 Apr 2013 21:45:31 -0300 Subject: [PATCH 4/4] Close all documents after each test and open them before each test. --- test/spec/EditorCommandHandlers-test.js | 44 +++++++++++++++---------- 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/test/spec/EditorCommandHandlers-test.js b/test/spec/EditorCommandHandlers-test.js index 022081abd57..df82b52d3ec 100644 --- a/test/spec/EditorCommandHandlers-test.js +++ b/test/spec/EditorCommandHandlers-test.js @@ -2092,27 +2092,35 @@ define(function (require, exports, module) { 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]; - }); } + + 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 () { - // Restore the content - editor.document.setText(moveContent); + 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"); + } + }); }); @@ -2131,7 +2139,7 @@ define(function (require, exports, module) { CommandManager.execute(Commands.EDIT_LINE_DOWN, editor); expect(editor.document.getText()).toEqual(moveContent); - expect(editor._codeMirror.doc.historySize().undo).toBe(1); + expect(editor._codeMirror.doc.historySize().undo).toBe(0); expect(editor.getFirstVisibleLine()).toBe(0); expect(editor.getLastVisibleLine()).toBe(2); });