Skip to content

Commit 82c672a

Browse files
committed
fix: stabilize JavaScript hints and desktop tests
Serialize queued Tern initializations and finish background indexing when directories disappear. Exclude HTML/PHP host markup from background parsing and supply extracted JavaScript with completion requests. Add initialization regressions, improve completion timeout diagnostics, and clean up fake-clock toast transitions. Pin Pro to the tested Design Mode, dialog, and locale fixes. Validated macOS Tauri categories: unit 2905, integration 1106 twice, LegacyInteg 556, livepreview 279, and mainview 307. Lint passed.
1 parent 74ae3cf commit 82c672a

6 files changed

Lines changed: 205 additions & 25 deletions

File tree

‎src/JSUtils/ScopeManager.js‎

Lines changed: 35 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -378,16 +378,31 @@ define(function (require, exports, module) {
378378
}
379379

380380
/**
381-
* Get the text of a document, applying any size restrictions
382-
* if necessary
381+
* Prepare a background file read or update for Tern's JavaScript parser.
382+
* HTML and PHP start empty: getFileInfo() supplies their extracted scripts with each hint
383+
* request. Parsing host markup here can throw and strand Tern's pending reads.
384+
* @private
385+
* @param {string} path - full path of the file.
386+
* @param {string} text - unfiltered file contents.
387+
* @return {string} JavaScript source, or an empty placeholder for HTML/PHP.
388+
*/
389+
function filterTernFileText(path, text) {
390+
const languageId = LanguageManager.getLanguageForPath(path).getId();
391+
if (languageId === "html" || languageId === "php") {
392+
return "";
393+
}
394+
return filterText(text);
395+
}
396+
397+
/**
398+
* Get a document's source for a background Tern update, excluding host markup
399+
* and applying the configured size limit.
383400
* @private
384401
* @param {Document} document - the document to get the text from
385-
* @return {string} the text, or the empty text if the original was too long
402+
* @return {string} JavaScript source, or empty text for HTML/PHP or an oversized file.
386403
*/
387404
function getTextFromDocument(document) {
388-
var text = document.getText();
389-
text = filterText(text);
390-
return text;
405+
return filterTernFileText(document.file.fullPath, document.getText());
391406
}
392407

393408
/**
@@ -606,14 +621,14 @@ define(function (require, exports, module) {
606621
* @return {{type: string, name: string, offsetLines: number, text: string}}
607622
*/
608623
function getFileInfo(session, preventPartialUpdates) {
624+
const languageId = LanguageManager.getLanguageForPath(session.editor.document.file.fullPath).getId();
609625
var start = session.getCursor(),
610626
end = start,
611627
document = session.editor.document,
612628
path = document.file.fullPath,
613-
isHtmlFile = LanguageManager.getLanguageForPath(path).getId() === "html",
614629
result;
615630

616-
if (isHtmlFile) {
631+
if (languageId === "html" || languageId === "php") {
617632
result = {type: MessageIds.TERN_FILE_INFO_TYPE_FULL,
618633
name: path,
619634
text: session.getJavascriptText()};
@@ -946,7 +961,7 @@ define(function (require, exports, module) {
946961
promise.done(function (docText) {
947962
resolvedFiles[name] = filePath;
948963
numResolvedFiles++;
949-
replyWith(name, filterText(docText));
964+
replyWith(name, filterTernFileText(filePath, docText));
950965
});
951966
return promise;
952967
}
@@ -1075,8 +1090,8 @@ define(function (require, exports, module) {
10751090
* to tern.
10761091
* @private
10771092
* @param {string} dir - the root directory to add.
1078-
* @param {function ()} doneCallback - called when all files have been
1079-
* added to tern.
1093+
* @param {function ()} doneCallback - called when traversal finishes, including
1094+
* when the directory can no longer be resolved.
10801095
*/
10811096
function addAllFilesAndSubdirectories(dir, doneCallback) {
10821097
FileSystem.resolve(dir, function (err, directory) {
@@ -1093,6 +1108,9 @@ define(function (require, exports, module) {
10931108
}
10941109

10951110
if (err) {
1111+
// The directory can disappear before background indexing starts.
1112+
// Release initialization so later editors do not wait on it indefinitely.
1113+
doneCallback();
10961114
return;
10971115
}
10981116

@@ -1326,12 +1344,14 @@ define(function (require, exports, module) {
13261344
* @param {?Document} previousDocument - the document of the editor is changing from
13271345
*/
13281346
function handleEditorChange(session, document, previousDocument) {
1329-
if (addFilesPromise === null) {
1330-
doEditorChange(session, document, previousDocument);
1331-
} else {
1347+
if (addFilesPromise && addFilesPromise.state() === "pending") {
13321348
addFilesPromise.done(function () {
1333-
doEditorChange(session, document, previousDocument);
1349+
// An earlier queued editor may have started another initialization.
1350+
// Recheck the current promise so only one worker-ready handler owns it.
1351+
handleEditorChange(session, document, previousDocument);
13341352
});
1353+
} else {
1354+
doEditorChange(session, document, previousDocument);
13351355
}
13361356
}
13371357

‎src/JSUtils/Session.js‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -742,15 +742,16 @@ define(function (require, exports, module) {
742742

743743
/**
744744
* Get the javascript text of the file open in the editor for this Session.
745-
* For a javascript file, this is just the text of the file. For an HTML file,
745+
* For a javascript file, this is just the text of the file. For an HTML/PHP file,
746746
* this will be only the text in the script tags. This is so that we can pass
747747
* just the javascript text to tern, and avoid confusing it with HTML tags, since it
748748
* only knows how to parse javascript.
749749
* @return {String} - the "javascript" text that can be sent to Tern.
750750
*/
751751
Session.prototype.getJavascriptText = function () {
752-
if (LanguageManager.getLanguageForPath(this.editor.document.file.fullPath).getId() === "html") {
753-
// HTML file - need to send back only the bodies of the
752+
const languageId = LanguageManager.getLanguageForPath(this.editor.document.file.fullPath).getId();
753+
if (languageId === "html" || languageId === "php") {
754+
// HTML/PHP file - need to send back only the bodies of the
754755
// script tags
755756
var text = "",
756757
editor = this.editor,

‎src/extensions/default/TypeScriptSupport/unittests.js‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -430,24 +430,28 @@ define(function (require, exports, module) {
430430
const jsCodeHints = await new Promise(function (resolve, reject) {
431431
ExtensionLoader.getRequireContextForExtension("JavaScriptCodeHints")(["main"], resolve, reject);
432432
});
433-
let hintText = "";
433+
let hintText = "", hintState = "not requested";
434434
await awaitsFor(async function () {
435435
if (!jsCodeHints.jsHintProvider.hasHints(editor, null)) {
436+
hintState = "provider declined the editor context";
436437
return false; // Tern session/worker may still be starting up
437438
}
438439
// The provider returns a Deferred for fresh hints and an object for
439440
// cached hints. Await either result before checking this request.
440441
let result;
441442
try {
443+
hintState = "waiting for the Tern response";
442444
result = await jsCodeHints.jsHintProvider.getHints(null);
443445
} catch (err) {
446+
hintState = "request rejected: " + String(err);
444447
return false; // A project/session change can cancel a pending request.
445448
}
446449
hintText = ((result && result.hints) || []).map(function (h) {
447450
return $(h).text();
448451
}).join("|");
452+
hintState = "returned hints: " + hintText;
449453
return hintText.indexOf("push") !== -1;
450-
}, "Tern Array-member completions at arr. inside the <script>", 30000, 500);
454+
}, () => "Tern Array-member completions at arr. inside the <script>: " + hintState, 30000, 500);
451455
expect(hintText).toContain("push");
452456

453457
await awaitsForDone(CommandManager.execute(Commands.FILE_CLOSE, { _forceClose: true }),

‎test/spec/NotificationUI-test.js‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,10 +110,11 @@ define(function (require, exports, module) {
110110
// Mock time from the start so the auto-close timer itself is under the clock. The close
111111
// itself is observed on the notification, since the removal waits for a CSS transition.
112112
jasmine.clock().install();
113+
let notification, $popup;
113114
try {
114-
const notification = NotificationUI.createToastFromTemplate("hello", "world", {autoCloseTimeS: 1});
115+
notification = NotificationUI.createToastFromTemplate("hello", "world", {autoCloseTimeS: 1});
116+
$popup = notification.$notification;
115117
spyOn(notification, "close").and.callThrough();
116-
const $popup = $("#toast-notification-container").children().last();
117118
expect($popup.length).toBe(1);
118119
$popup.trigger("mouseenter");
119120
jasmine.clock().tick(1500);
@@ -125,6 +126,13 @@ define(function (require, exports, module) {
125126
jasmine.clock().tick(200);
126127
expect(notification.close).toHaveBeenCalledWith(NotificationUI.CLOSE_REASON.TIMEOUT);
127128
} finally {
129+
if (notification) {
130+
notification.close("test-cleanup");
131+
// The fake clock does not advance CSS transitions. Complete this
132+
// toast's removal so the next spec starts without a closing toast.
133+
$popup.trigger("transitionend");
134+
expect($popup[0].isConnected).toBeFalse();
135+
}
128136
jasmine.clock().uninstall();
129137
}
130138
});

‎test/spec/ScopeManager-integ-test.js‎

Lines changed: 149 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,17 +7,19 @@
77

88
define(function (require, exports, module) {
99
const SpecRunnerUtils = require("spec/SpecRunnerUtils"),
10-
FileSystemError = require("filesystem/FileSystemError");
10+
FileSystemError = require("filesystem/FileSystemError"),
11+
MessageIds = JSON.parse(require("text!JSUtils/MessageIds.json"));
1112

1213
describe("integration:JavaScript hint initialization", function () {
13-
let testWindow, FileSystem, ScopeManager, EditorManager, Session, testFolder;
14+
let testWindow, FileSystem, ScopeManager, EditorManager, Session, IndexingWorker, testFolder;
1415

1516
beforeAll(async function () {
1617
testWindow = await SpecRunnerUtils.createTestWindowAndRun();
1718
FileSystem = testWindow.brackets.test.FileSystem;
1819
EditorManager = testWindow.brackets.test.EditorManager;
1920
ScopeManager = testWindow.brackets.getModule("JSUtils/ScopeManager");
2021
Session = testWindow.brackets.getModule("JSUtils/Session");
22+
IndexingWorker = testWindow.brackets.getModule("worker/IndexingWorker");
2123
}, 30000);
2224

2325
beforeEach(async function () {
@@ -95,5 +97,150 @@ define(function (require, exports, module) {
9597
reads.restore();
9698
}
9799
});
100+
101+
it("should finish initialization when a background directory disappears", async function () {
102+
await SpecRunnerUtils.createTextFileAsync(testFolder + "/hints.js", "const arr = [1, 2, 3];\narr.");
103+
await awaitsForDone(SpecRunnerUtils.openProjectFiles(["hints.js"]), "open JavaScript fixture");
104+
const editor = EditorManager.getCurrentFullEditor();
105+
editor.setCursorPos(1, 4);
106+
const session = new Session(editor);
107+
await awaitsForDone(ScopeManager.requestHints(session, editor.document), "initialize Tern");
108+
109+
const directoryPath = editor.document.file.parentPath;
110+
const resolve = FileSystem.resolve;
111+
let directoryReads = 0, interruptedLookup;
112+
const lookup = spyOn(FileSystem, "resolve").and.callFake(function (filePath, callback) {
113+
if (filePath === directoryPath && ++directoryReads === 2) {
114+
// The first lookup initializes Tern. The second starts its background scan,
115+
// after the directory could have been deleted during a project switch.
116+
interruptedLookup = callback;
117+
callback(FileSystemError.NOT_FOUND);
118+
return;
119+
}
120+
return resolve.apply(this, arguments);
121+
});
122+
const ready = ScopeManager._maybeReset(session, editor.document, true);
123+
try {
124+
await awaitsFor(() => !!interruptedLookup, "background directory lookup");
125+
await awaitsFor(() => ready.state() !== "pending", "initialization after a missing directory", 5000);
126+
await awaitsForDone(ready, "initialization after a missing directory");
127+
const hints = ScopeManager.requestHints(session, editor.document);
128+
await awaitsForDone(hints, "hints after a missing directory");
129+
expect(session.ternHints.some(hint => hint.value === "push")).toBeTrue();
130+
} finally {
131+
lookup.and.callThrough();
132+
if (interruptedLookup && ready.state() === "pending") {
133+
// Release the injected failure even if an assertion fails, so subsequent
134+
// specs are not left behind the same unfinished initialization.
135+
resolve.call(FileSystem, directoryPath, interruptedLookup);
136+
await awaitsForDone(ready, "finish test cleanup");
137+
}
138+
}
139+
});
140+
141+
it("should initialize HTML without parsing markup as JavaScript", async function () {
142+
const worker = spyOn(IndexingWorker, "execPeer").and.callThrough();
143+
await awaitsForDone(SpecRunnerUtils.openProjectFiles(["embedded.html"]), "open embedded script");
144+
const editor = EditorManager.getCurrentFullEditor();
145+
editor.setCursorPos(6, 4);
146+
const session = new Session(editor);
147+
await awaitsForDone(ScopeManager.requestHints(session, editor.document), "embedded script hints");
148+
149+
const reads = worker.calls.allArgs().filter(args => args[0] === "invokeTernCommand" &&
150+
args[1].type === MessageIds.TERN_GET_FILE_MSG && args[1].file.endsWith("/embedded.html"));
151+
expect(reads.length).toBeGreaterThan(0);
152+
reads.forEach(args => expect(args[1].text).not.toContain("<html>"));
153+
expect(session.ternHints.some(hint => hint.value === "push")).toBeTrue();
154+
});
155+
156+
it("should serialize editor changes queued behind initialization", async function () {
157+
await awaitsForDone(SpecRunnerUtils.openProjectFiles(["embedded.html"]), "open embedded script");
158+
const editor = EditorManager.getCurrentFullEditor();
159+
editor.setCursorPos(6, 4);
160+
const session = new Session(editor);
161+
await awaitsForDone(ScopeManager.requestHints(session, editor.document), "initialize Tern");
162+
163+
const documents = [];
164+
for (const name of ["queued-a.html", "queued-b.html", "queued-c.html"]) {
165+
const path = testFolder + "/" + name;
166+
await SpecRunnerUtils.createTextFileAsync(path, editor.document.getText());
167+
documents.push(await testWindow.brackets.test.DocumentManager.getDocumentForPath(path));
168+
}
169+
170+
const directoryPath = editor.document.file.parentPath;
171+
const resolve = FileSystem.resolve;
172+
const callbacks = [];
173+
const lookup = spyOn(FileSystem, "resolve").and.callFake(function (filePath, callback) {
174+
if (filePath === directoryPath) {
175+
callbacks.push(callback);
176+
return;
177+
}
178+
return resolve.apply(this, arguments);
179+
});
180+
try {
181+
documents.forEach(document => ScopeManager.handleEditorChange(session, document, null));
182+
expect(callbacks.length).toBe(1);
183+
resolve.call(FileSystem, directoryPath, callbacks.shift());
184+
await awaitsFor(() => callbacks.length > 0, "next queued editor initialization");
185+
// Waking several queued changes must start only the next initialization.
186+
// Concurrent initializations replace each other's worker-ready handler.
187+
expect(callbacks.length).toBe(1);
188+
} finally {
189+
// Fail held lookups before restoring them, including any cleanup queues.
190+
while (callbacks.length) {
191+
callbacks.shift()(FileSystemError.NOT_FOUND);
192+
}
193+
lookup.and.callThrough();
194+
}
195+
196+
await awaitsForDone(SpecRunnerUtils.openProjectFiles(["queued-c.html"]), "open last queued editor");
197+
const finalEditor = EditorManager.getCurrentFullEditor();
198+
finalEditor.setCursorPos(6, 4);
199+
const finalSession = new Session(finalEditor);
200+
await awaitsForDone(ScopeManager.requestHints(finalSession, finalEditor.document), "hints after queued editors");
201+
expect(finalSession.ternHints.some(hint => hint.value === "push")).toBeTrue();
202+
});
203+
204+
it("should exclude markup when updating a dirty HTML document before switching editors", async function () {
205+
await awaitsForDone(SpecRunnerUtils.openProjectFiles(["embedded.html"]), "open embedded script");
206+
const previousEditor = EditorManager.getCurrentFullEditor();
207+
previousEditor.setCursorPos(6, 4);
208+
await awaitsForDone(ScopeManager.requestHints(new Session(previousEditor), previousEditor.document),
209+
"initialize embedded script");
210+
await SpecRunnerUtils.createTextFileAsync(testFolder + "/other.html", previousEditor.document.getText());
211+
212+
const worker = spyOn(IndexingWorker, "execPeer").and.callThrough();
213+
previousEditor.document.setText(previousEditor.document.getText() + "\n<!-- unsaved edit -->");
214+
await awaitsForDone(SpecRunnerUtils.openProjectFiles(["other.html"]), "switch embedded script");
215+
const editor = EditorManager.getCurrentFullEditor();
216+
editor.setCursorPos(6, 4);
217+
const session = new Session(editor);
218+
await awaitsForDone(ScopeManager.requestHints(session, editor.document), "hints after dirty HTML update");
219+
220+
const updates = worker.calls.allArgs().filter(args => args[0] === "invokeTernCommand" &&
221+
args[1].type === MessageIds.TERN_UPDATE_FILE_MSG &&
222+
args[1].path === previousEditor.document.file.fullPath);
223+
expect(updates.length).toBeGreaterThan(0);
224+
updates.forEach(args => expect(args[1].text).not.toContain("<html>"));
225+
expect(session.ternHints.some(hint => hint.value === "push")).toBeTrue();
226+
});
227+
228+
it("should analyze embedded JavaScript without sending PHP host code to Tern", async function () {
229+
const html = await SpecRunnerUtils.readTextFileAsync(testFolder + "/embedded.html");
230+
await SpecRunnerUtils.createTextFileAsync(testFolder + "/embedded.php", '<?php echo "header"; ?>\n' + html);
231+
const worker = spyOn(IndexingWorker, "execPeer").and.callThrough();
232+
await awaitsForDone(SpecRunnerUtils.openProjectFiles(["embedded.php"]), "open PHP embedded script");
233+
const editor = EditorManager.getCurrentFullEditor();
234+
editor.setCursorPos(7, 4);
235+
const session = new Session(editor);
236+
await awaitsForDone(ScopeManager.requestHints(session, editor.document), "PHP embedded script hints");
237+
238+
const reads = worker.calls.allArgs().filter(args => args[0] === "invokeTernCommand" &&
239+
args[1].type === MessageIds.TERN_GET_FILE_MSG && args[1].file.endsWith("/embedded.php"));
240+
expect(reads.length).toBeGreaterThan(0);
241+
reads.forEach(args => expect(args[1].text).not.toContain("<?php"));
242+
expect(session.ternHints.some(hint => hint.value === "push")).toBeTrue();
243+
});
244+
98245
});
99246
});

‎tracking-repos.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
{
22
"phoenixPro": {
3-
"commitID": "24695997878195f3499c60e453060591f6079b18"
3+
"commitID": "c991a453739841abeeacf7711127df553e39578e"
44
}
55
}

0 commit comments

Comments
 (0)