Skip to content
This repository was archived by the owner on Sep 6, 2021. It is now read-only.

Commit df1d141

Browse files
author
Narciso Jaramillo
committed
Add contentChange event to TextRange. Make it so original change event doesn't fire if boundaries don't actually change.
1 parent 607b35d commit df1d141

3 files changed

Lines changed: 206 additions & 15 deletions

File tree

src/document/TextRange.js

Lines changed: 31 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,10 @@ define(function (require, exports, module) {
4040
* Important: you must dispose() a TextRange when you're done with it. Because TextRange addRef()s
4141
* the Document (in order to listen to it), you will leak Documents otherwise.
4242
*
43-
* TextRange dispatches two events:
44-
* - change -- When the range changes (due to a Document change)
43+
* TextRange dispatches these events:
44+
* - change -- When the range boundary line numbers change (due to a Document change)
45+
* - contentChange -- When the actual content of the range changes. This might or might not
46+
* be accompanied by a change in the boundary line numbers.
4547
* - lostSync -- When the backing Document changes in such a way that the range can no longer
4648
* accurately be maintained. Generally, occurs whenever an edit spans a range boundary.
4749
* After this, startLine & endLine will be unusable (set to null).
@@ -86,7 +88,10 @@ define(function (require, exports, module) {
8688

8789
/**
8890
* Applies a single Document change object (out of the linked list of multiple such objects)
89-
* to this range. Returns true if the range was changed as a result.
91+
* to this range.
92+
* @param {Object} change The CodeMirror change record.
93+
* @return {{hasChanged: boolean, hasContentChanged: boolean}} Whether the range boundary
94+
* and/or content has changed.
9095
*/
9196
TextRange.prototype._applySingleChangeToRange = function (change) {
9297
// console.log(this + " applying change to (" +
@@ -97,7 +102,7 @@ define(function (require, exports, module) {
97102
if (!change.from || !change.to) {
98103
this.startLine = null;
99104
this.endLine = null;
100-
return true;
105+
return {hasChanged: true, hasContentChanged: true};
101106

102107
// Special case: certain changes around the edges of the range are problematic, because
103108
// if they're undone, we'll be unable to determine how to fix up the range to include the
@@ -116,29 +121,36 @@ define(function (require, exports, module) {
116121
(change.from.line <= this.endLine && change.to.line > this.endLine)) {
117122
this.startLine = null;
118123
this.endLine = null;
119-
return true;
124+
return {hasChanged: true, hasContentChanged: true};
120125

121126
// Normal case: update the range end points if any content was added before them. Note that
122127
// we don't rely on line handles for this since we want to gracefully handle cases where the
123128
// start or end line was deleted during a change.
124129
} else {
125130
var numAdded = change.text.length - (change.to.line - change.from.line + 1);
126-
var hasChanged = false;
131+
var result = {hasChanged: false, hasContentChanged: false};
127132

128133
// This logic is so simple because we've already excluded all cases where the change
129134
// crosses the range boundaries
130-
if (change.to.line < this.startLine) {
131-
this.startLine += numAdded;
132-
hasChanged = true;
135+
if (numAdded !== 0) {
136+
if (change.to.line < this.startLine) {
137+
this.startLine += numAdded;
138+
result.hasChanged = true;
139+
}
140+
if (change.to.line <= this.endLine) {
141+
this.endLine += numAdded;
142+
result.hasChanged = true;
143+
}
133144
}
134-
if (change.to.line <= this.endLine) {
135-
this.endLine += numAdded;
136-
hasChanged = true;
145+
if (change.from.line >= this.startLine && change.from.line <= this.endLine) {
146+
// Since we know the change doesn't cross the range boundary, as long as the
147+
// start of the change is within the range, we know the content changed.
148+
result.hasContentChanged = true;
137149
}
138150

139151
// console.log("Now " + this);
140152

141-
return hasChanged;
153+
return result;
142154
}
143155
};
144156

@@ -148,12 +160,13 @@ define(function (require, exports, module) {
148160
* range can no longer be accurately maintained.
149161
*/
150162
TextRange.prototype._applyChangesToRange = function (changeList) {
151-
var hasChanged = false;
163+
var hasChanged = false, hasContentChanged = false;
152164
var change;
153165
for (change = changeList; change; change = change.next) {
154166
// Apply this step of the change list
155167
var result = this._applySingleChangeToRange(change);
156-
hasChanged = hasChanged || result;
168+
hasChanged = hasChanged || result.hasChanged;
169+
hasContentChanged = hasContentChanged || result.hasContentChanged;
157170

158171
// If we lost sync with the range, just bail now
159172
if (this.startLine === null || this.endLine === null) {
@@ -165,6 +178,9 @@ define(function (require, exports, module) {
165178
if (hasChanged) {
166179
$(this).triggerHandler("change");
167180
}
181+
if (hasContentChanged) {
182+
$(this).triggerHandler("contentChange");
183+
}
168184
};
169185

170186
TextRange.prototype._handleDocumentChange = function (event, doc, changeList) {

test/UnitTestSuite.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ define(function (require, exports, module) {
6464
require("spec/QuickOpen-test");
6565
require("spec/RemoteFunctions-test");
6666
require("spec/StringMatch-test");
67+
require("spec/TextRange-test");
6768
require("spec/UpdateNotification-test");
6869
require("spec/ViewCommandHandlers-test");
6970
require("spec/ViewUtils-test");

test/spec/TextRange-test.js

Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,174 @@
1+
/*
2+
* Copyright (c) 2013 Adobe Systems Incorporated. All rights reserved.
3+
*
4+
* Permission is hereby granted, free of charge, to any person obtaining a
5+
* copy of this software and associated documentation files (the "Software"),
6+
* to deal in the Software without restriction, including without limitation
7+
* the rights to use, copy, modify, merge, publish, distribute, sublicense,
8+
* and/or sell copies of the Software, and to permit persons to whom the
9+
* Software is furnished to do so, subject to the following conditions:
10+
*
11+
* The above copyright notice and this permission notice shall be included in
12+
* all copies or substantial portions of the Software.
13+
*
14+
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
15+
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
16+
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
17+
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
18+
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
19+
* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
20+
* DEALINGS IN THE SOFTWARE.
21+
*
22+
*/
23+
24+
25+
/*jslint vars: true, plusplus: true, devel: true, browser: true, nomen: true, indent: 4, maxerr: 50 */
26+
/*global define, describe, it, expect, beforeEach, afterEach, $ */
27+
28+
define(function (require, exports, module) {
29+
"use strict";
30+
31+
var TextRange = require("document/TextRange").TextRange,
32+
SpecRunnerUtils = require("spec/SpecRunnerUtils");
33+
34+
var docText = "", i;
35+
for (i = 0; i < 10; i++) {
36+
docText += "line " + i + "\n";
37+
}
38+
39+
describe("TextRange", function () {
40+
var doc,
41+
range,
42+
gotChange,
43+
gotContentChange,
44+
gotLostSync;
45+
46+
beforeEach(function () {
47+
var result = SpecRunnerUtils.createMockEditor(docText);
48+
doc = result.doc;
49+
50+
gotChange = false;
51+
gotContentChange = false;
52+
gotLostSync = false;
53+
54+
range = new TextRange(doc, 4, 6);
55+
$(range).on("change.unittest", function () {
56+
expect(gotChange).toBe(false);
57+
gotChange = true;
58+
}).on("contentChange.unittest", function () {
59+
expect(gotContentChange).toBe(false);
60+
gotContentChange = true;
61+
}).on("lostSync.unittest", function () {
62+
expect(gotLostSync).toBe(false);
63+
gotLostSync = true;
64+
});
65+
});
66+
67+
afterEach(function () {
68+
SpecRunnerUtils.destroyMockEditor(doc);
69+
doc = null;
70+
$(range).off(".unittest");
71+
range.dispose();
72+
range = null;
73+
});
74+
75+
it("should not update or fire events for an edit before that doesn't change the number of lines", function () {
76+
doc.replaceRange("new line 2\nnew line 3", {line: 2, ch: 0}, {line: 3, ch: 6});
77+
expect(gotChange).toBe(false);
78+
expect(gotContentChange).toBe(false);
79+
expect(gotLostSync).toBe(false);
80+
expect(range.startLine).toBe(4);
81+
expect(range.endLine).toBe(6);
82+
});
83+
84+
it("should update and fire a change, but not a contentChange, for an edit before that deletes a line", function () {
85+
doc.replaceRange("", {line: 2, ch: 0}, {line: 3, ch: 0});
86+
expect(gotChange).toBe(true);
87+
expect(gotContentChange).toBe(false);
88+
expect(gotLostSync).toBe(false);
89+
expect(range.startLine).toBe(3);
90+
expect(range.endLine).toBe(5);
91+
});
92+
93+
it("should update and fire a change, but not a contentChange, for an edit before that inserts a line", function () {
94+
doc.replaceRange("new extra line\n", {line: 2, ch: 0});
95+
expect(gotChange).toBe(true);
96+
expect(gotContentChange).toBe(false);
97+
expect(gotLostSync).toBe(false);
98+
expect(range.startLine).toBe(5);
99+
expect(range.endLine).toBe(7);
100+
});
101+
102+
it("should not update or fire events for an edit after even if it changes the number of lines", function () {
103+
doc.replaceRange("new extra line\n", {line: 8, ch: 0});
104+
expect(gotChange).toBe(false);
105+
expect(gotContentChange).toBe(false);
106+
expect(gotLostSync).toBe(false);
107+
expect(range.startLine).toBe(4);
108+
expect(range.endLine).toBe(6);
109+
});
110+
111+
it("should lose sync if entire document is replaced", function () {
112+
doc.replaceRange("new content", {line: 0, ch: 0}, {line: 9, ch: 6});
113+
expect(gotChange).toBe(true);
114+
expect(gotContentChange).toBe(true);
115+
expect(gotLostSync).toBe(true);
116+
expect(range.startLine).toBe(null);
117+
expect(range.endLine).toBe(null);
118+
});
119+
120+
it("should lose sync if entire range is replaced", function () {
121+
doc.replaceRange("new content", {line: 3, ch: 0}, {line: 7, ch: 6});
122+
expect(gotChange).toBe(true);
123+
expect(gotContentChange).toBe(true);
124+
expect(gotLostSync).toBe(true);
125+
expect(range.startLine).toBe(null);
126+
expect(range.endLine).toBe(null);
127+
});
128+
129+
it("should lose sync if a change overlaps the beginning of the range", function () {
130+
doc.replaceRange("new content", {line: 3, ch: 0}, {line: 5, ch: 6});
131+
expect(gotChange).toBe(true);
132+
expect(gotContentChange).toBe(true);
133+
expect(gotLostSync).toBe(true);
134+
expect(range.startLine).toBe(null);
135+
expect(range.endLine).toBe(null);
136+
});
137+
138+
it("should lose sync if a change overlaps the end of the range", function () {
139+
doc.replaceRange("new content", {line: 5, ch: 0}, {line: 7, ch: 6});
140+
expect(gotChange).toBe(true);
141+
expect(gotContentChange).toBe(true);
142+
expect(gotLostSync).toBe(true);
143+
expect(range.startLine).toBe(null);
144+
expect(range.endLine).toBe(null);
145+
});
146+
147+
it("should not update or send a change, but should send contentChange, if a change occurs inside range without changing # of lines", function () {
148+
doc.replaceRange("new line 5", {line: 5, ch: 0}, {line: 5, ch: 6});
149+
expect(gotChange).toBe(false);
150+
expect(gotContentChange).toBe(true);
151+
expect(gotLostSync).toBe(false);
152+
expect(range.startLine).toBe(4);
153+
expect(range.endLine).toBe(6);
154+
});
155+
156+
it("should update and send change/contentChange if a line is added inside range", function () {
157+
doc.replaceRange("new added line\n", {line: 5, ch: 0});
158+
expect(gotChange).toBe(true);
159+
expect(gotContentChange).toBe(true);
160+
expect(gotLostSync).toBe(false);
161+
expect(range.startLine).toBe(4);
162+
expect(range.endLine).toBe(7);
163+
});
164+
165+
it("should update and send change/contentChange if a line is deleted inside range", function () {
166+
doc.replaceRange("", {line: 5, ch: 0}, {line: 6, ch: 0});
167+
expect(gotChange).toBe(true);
168+
expect(gotContentChange).toBe(true);
169+
expect(gotLostSync).toBe(false);
170+
expect(range.startLine).toBe(4);
171+
expect(range.endLine).toBe(5);
172+
});
173+
});
174+
});

0 commit comments

Comments
 (0)