Skip to content

Commit

Permalink
fix(extensions/#3215): Part 1 - Implement max count parameter for fin…
Browse files Browse the repository at this point in the history 
…dFiles API (#3241)

__Issue:__ Some extensions that use the `vscode.workspace.findFiles` API wouldn't work as expected - like the [Live Server](https://open-vsx.org/extension/ritwickdey/LiveServer) extension.

__Defect:__ The `maxCount` argument wasn't being used by Onivim - so for a very large folder, we might not finish traversing for a while. In addition, the `readdir` call could fail, in which case no result would be returned at at all.
 
__Fix:__ Implement handling of `maxCount`, and short-circuit the find-in-files once the limit is reached. This fixes the first blocking issue for #3215
  • Loading branch information
@bryphe
bryphe authored Mar 8, 2021
1 parent 4431f88 commit 9982221
Show file tree
Hide file tree
Showing 7 changed files with 225 additions and 114 deletions.
1 change: 1 addition & 0 deletions CHANGES_CURRENT.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
- #3233 - Formatting: Fix buffer de-sync when applying formatting edits (fixes #2196, #2820)
- #3239 - Buffers: Fix filetype picker not working as expected without an active workspace
- #3240 - Formatting: Fix 'Invalid Range Specified' error (fixes #3014)
- #3241 - Extensions: Handle `maxCount` and FS errors in `vscode.workspace.findFiles` (related #3215)

### Performance

Expand Down
97 changes: 54 additions & 43 deletions development_extensions/oni-dev-extension/extension.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,18 @@ function activate(context) {
vscode.window.showWarningMessage("You clicked developer", [])
}),
)


cleanup(
vscode.commands.registerCommand("_test.findFiles", (count) => {
vscode.workspace.findFiles(
"*.json", undefined, count
).then((results) => {
vscode.window.showInformationMessage("success:" + results.length);
// vscode.window.showInformationMessage("success:1");
})
}),
)

cleanup(
vscode.commands.registerCommand("developer.oni.hideStatusBar", () => {
item.hide();
Expand All @@ -37,31 +48,31 @@ function activate(context) {

cleanup(
vscode.commands.registerCommand("developer.oni.logBufferUpdates", () => {
vscode.window.showInformationMessage("Logging buffer updates!");
vscode.workspace.onDidChangeTextDocument((e) => {
console.log({
type: "workspace.onDidChangeTextDocument",
filename: e.document.fileName,
version: e.document.version,
contentChanges: e.contentChanges,
fullText: e.document.getText(),
});
vscode.window.showInformationMessage("Logging buffer updates!");
vscode.workspace.onDidChangeTextDocument((e) => {
console.log({
type: "workspace.onDidChangeTextDocument",
filename: e.document.fileName,
version: e.document.version,
contentChanges: e.contentChanges,
fullText: e.document.getText(),
});
});
}),
)

cleanup(
vscode.commands.registerCommand("developer.oni.tryOpenDocument", () => {
vscode.workspace.openTextDocument(vscode.Uri.file("package.json"))
.then((document) => {
let text = document.getText();
vscode.window.showInformationMessage("Got text document: " + text);
}, err => {
vscode.window.showErrorMessage("Failed to get text document: " + err.toString());
})
.then((document) => {
let text = document.getText();
vscode.window.showInformationMessage("Got text document: " + text);
}, err => {
vscode.window.showErrorMessage("Failed to get text document: " + err.toString());
})
}),
)

cleanup(
vscode.commands.registerCommand("developer.oni.showStatusBar", () => {
item.show();
Expand All @@ -83,7 +94,7 @@ function activate(context) {
return [vscode.CompletionItem("HelloWorld"), vscode.CompletionItem("HelloAgain")]
},
resolveCompletionItem: (completionItem, token) => {
completionItem.documentation = "RESOLVED documentation: "+ completionItem.label;
completionItem.documentation = "RESOLVED documentation: " + completionItem.label;
completionItem.detail = "RESOLVED detail: " + completionItem.label;
return completionItem;
}
Expand Down Expand Up @@ -120,10 +131,10 @@ function activate(context) {
return new Promise((resolve) => {
setTimeout(() => {
resolve([
vscode.CompletionItem("ReasonML0"),
vscode.CompletionItem("OCaml0"),
vscode.CompletionItem("ReasonML2"),
vscode.CompletionItem("OCaml2"),
vscode.CompletionItem("ReasonML0"),
vscode.CompletionItem("OCaml0"),
vscode.CompletionItem("ReasonML2"),
vscode.CompletionItem("OCaml2"),
])
}, 500);
});
Expand All @@ -135,15 +146,15 @@ function activate(context) {
cleanup(
vscode.languages.registerCompletionItemProvider("oni-dev", {
provideCompletionItems: (document, position, token, context) => {
const items = [
vscode.CompletionItem("Incomplete" + Date.now().toString()),
];
return {
isIncomplete: true,
items,
};
},
}),
const items = [
vscode.CompletionItem("Incomplete" + Date.now().toString()),
];
return {
isIncomplete: true,
items,
};
},
}),
)

const output = vscode.window.createOutputChannel("oni-dev")
Expand All @@ -153,7 +164,7 @@ function activate(context) {
output2.append("Hello output channel!")

const collection = vscode.languages.createDiagnosticCollection("test")

let latestText = ""

cleanup(
Expand Down Expand Up @@ -209,16 +220,16 @@ function activate(context) {
cleanup(
vscode.commands.registerCommand("developer.oni.showChoiceMessage", () => {
vscode.window.showInformationMessage(
//`Sed ut perspiciatis unde omnis iste natus error sit voluptatem accusantium doloremque laudantium, totam rem aperiam, eaque ipsa quae ab illo inventore veritatis et quasi architecto beatae vitae dicta sunt explicabo. Nemo enim ipsam voluptatem quia voluptas sit aspernatur aut odit aut fugit, sed quia consequuntur magni dolores eos qui ratione voluptatem sequi nesciunt. Neque porro quisquam est, qui dolorem ipsum quia dolor sit amet, consectetur, adipisci velit, sed quia non numquam eius modi tempora incidunt ut labore et dolore magnam aliquam quaerat voluptatem. Ut enim ad minima veniam, quis nostrum exercitationem ullam corporis suscipit laboriosam, nisi ut aliquid ex ea commodi consequatur? Quis autem vel eum iure reprehenderit qui in ea voluptate velit esse quam nihil molestiae consequatur, vel illum qui dolorem eum fugiat quo voluptas nulla pariatur?`
"Hello!"
, "Option 1", "Option 2", "Option 3").then(
(result) => {
vscode.window.showInformationMessage("You picked: " + result)
},
(err) => {
vscode.window.showInformationMessage("Cancelled: " + err)
},
)
//`Sed ut perspiciatis unde omnis iste natus error sit voluptatem accusantium doloremque laudantium, totam rem aperiam, eaque ipsa quae ab illo inventore veritatis et quasi architecto beatae vitae dicta sunt explicabo. Nemo enim ipsam voluptatem quia voluptas sit aspernatur aut odit aut fugit, sed quia consequuntur magni dolores eos qui ratione voluptatem sequi nesciunt. Neque porro quisquam est, qui dolorem ipsum quia dolor sit amet, consectetur, adipisci velit, sed quia non numquam eius modi tempora incidunt ut labore et dolore magnam aliquam quaerat voluptatem. Ut enim ad minima veniam, quis nostrum exercitationem ullam corporis suscipit laboriosam, nisi ut aliquid ex ea commodi consequatur? Quis autem vel eum iure reprehenderit qui in ea voluptate velit esse quam nihil molestiae consequatur, vel illum qui dolorem eum fugiat quo voluptas nulla pariatur?`
"Hello!"
, "Option 1", "Option 2", "Option 3").then(
(result) => {
vscode.window.showInformationMessage("You picked: " + result)
},
(err) => {
vscode.window.showInformationMessage("Cancelled: " + err)
},
)
}),
)

Expand Down Expand Up @@ -392,7 +403,7 @@ function activate(context) {
}

// this method is called when your extension is deactivated
function deactivate() {}
function deactivate() { }

module.exports = {
activate,
Expand Down
69 changes: 69 additions & 0 deletions integration_test/ExtHostWorkspaceSearchTest.re
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
open Oni_Model;
open Oni_IntegrationTestLib;

// This test validates:
// - The 'oni-dev' extension gets activated
// - When typing in an 'oni-dev' buffer, we get some completion results
runTest(~name="ExtHostWorkspaceSearchTest", ({dispatch, wait, _}) => {
wait(~timeout=30.0, ~name="Exthost is initialized", (state: State.t) =>
Feature_Exthost.isInitialized(state.exthost)
);

// Wait until the extension is activated
// Give some time for the exthost to start
wait(
~timeout=30.0,
~name="Validate the 'oni-dev' extension gets activated",
(state: State.t) =>
List.exists(
id => id == "oni-dev-extension",
state.extensions |> Feature_Extensions.activatedIds,
)
);

// Search for 1 match
dispatch(
Actions.Extensions(
Feature_Extensions.Msg.command(
~command="_test.findFiles",
~arguments=[`Int(1)],
),
),
);

// Wait for a notification
wait(
~timeout=30.,
~name="Got success notification for 1 result",
(state: State.t) => {
let notifications = Feature_Notification.all(state.notifications);
notifications
|> List.exists((notification: Feature_Notification.notification) => {
notification.message == "success:1"
});
},
);

// Search for multiple matches
dispatch(
Actions.Extensions(
Feature_Extensions.Msg.command(
~command="_test.findFiles",
~arguments=[`Int(3)],
),
),
);

// Wait for a notification
wait(
~timeout=60.,
~name="Got success notification for 5 results",
(state: State.t) => {
let notifications = Feature_Notification.all(state.notifications);
notifications
|> List.exists((notification: Feature_Notification.notification) => {
notification.message == "success:3"
});
},
);
});
50 changes: 25 additions & 25 deletions integration_test/dune
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,24 +8,24 @@
EditorUtf8Test ExCommandKeybindingTest ExCommandKeybindingWithArgsTest
ExCommandKeybindingNormTest ExtConfigurationChangedTest
ExtHostBufferOpenTest ExtHostBufferUpdatesTest ExtHostCompletionTest
ExtHostDefinitionTest FileWatcherTest KeybindingsInvalidJsonTest
KeySequenceJJTest InputIgnoreTest InputContextKeysTest
InputRemapMotionTest LanguageCssTest LanguageTypeScriptTest
LineEndingsLFTest LineEndingsCRLFTest QuickOpenEventuallyCompletesTest
SearchShowClearHighlightsTest SyntaxServer SyntaxServerCloseTest
SyntaxServerMessageExceptionTest SyntaxServerParentPidTest
SyntaxServerParentPidCorrectTest SyntaxServerReadExceptionTest
Regression1671Test Regression2349EnewTest Regression2988SwitchEditorTest
Regression3084CommandLoopTest RegressionCommandLineNoCompletionTest
RegressionFontFallbackTest RegressionFileModifiedIndicationTest
RegressionNonExistentDirectory RegressionVspEmptyInitialBufferTest
RegressionVspEmptyExistingBufferTest SCMGitTest SyntaxHighlightPhpTest
SyntaxHighlightTextMateTest SyntaxHighlightTreesitterTest
AddRemoveSplitTest TerminalSetPidTitleTest TerminalConfigurationTest
TypingBatchedTest TypingUnbatchedTest VimIncsearchScrollTest
VimExperimentalVimlTest VimRemapTimeoutTest VimSimpleRemapTest
VimlRemapCmdlineTest ClipboardChangeTest VimScriptLocalFunctionTest
ZenModeSingleFileModeTest ZenModeSplitTest)
ExtHostDefinitionTest ExtHostWorkspaceSearchTest FileWatcherTest
KeybindingsInvalidJsonTest KeySequenceJJTest InputIgnoreTest
InputContextKeysTest InputRemapMotionTest LanguageCssTest
LanguageTypeScriptTest LineEndingsLFTest LineEndingsCRLFTest
QuickOpenEventuallyCompletesTest SearchShowClearHighlightsTest
SyntaxServer SyntaxServerCloseTest SyntaxServerMessageExceptionTest
SyntaxServerParentPidTest SyntaxServerParentPidCorrectTest
SyntaxServerReadExceptionTest Regression1671Test Regression2349EnewTest
Regression2988SwitchEditorTest Regression3084CommandLoopTest
RegressionCommandLineNoCompletionTest RegressionFontFallbackTest
RegressionFileModifiedIndicationTest RegressionNonExistentDirectory
RegressionVspEmptyInitialBufferTest RegressionVspEmptyExistingBufferTest
SCMGitTest SyntaxHighlightPhpTest SyntaxHighlightTextMateTest
SyntaxHighlightTreesitterTest AddRemoveSplitTest TerminalSetPidTitleTest
TerminalConfigurationTest TypingBatchedTest TypingUnbatchedTest
VimIncsearchScrollTest VimExperimentalVimlTest VimRemapTimeoutTest
VimSimpleRemapTest VimlRemapCmdlineTest ClipboardChangeTest
VimScriptLocalFunctionTest ZenModeSingleFileModeTest ZenModeSplitTest)
(libraries Oni_CLI Oni_IntegrationTestLib reason-native-crash-utils.asan))

(install
Expand All @@ -43,13 +43,13 @@
ExCommandKeybindingNormTest.exe ExCommandKeybindingWithArgsTest.exe
ExtConfigurationChangedTest.exe ExtHostBufferOpenTest.exe
ExtHostBufferUpdatesTest.exe ExtHostCompletionTest.exe
ExtHostDefinitionTest.exe FileWatcherTest.exe
KeybindingsInvalidJsonTest.exe KeySequenceJJTest.exe InputIgnoreTest.exe
InputContextKeysTest.exe InputRemapMotionTest.exe LanguageCssTest.exe
LanguageTypeScriptTest.exe LineEndingsCRLFTest.exe LineEndingsLFTest.exe
QuickOpenEventuallyCompletesTest.exe Regression1671Test.exe
Regression2349EnewTest.exe Regression2988SwitchEditorTest.exe
Regression3084CommandLoopTest.exe
ExtHostDefinitionTest.exe ExtHostWorkspaceSearchTest.exe
FileWatcherTest.exe KeybindingsInvalidJsonTest.exe KeySequenceJJTest.exe
InputIgnoreTest.exe InputContextKeysTest.exe InputRemapMotionTest.exe
LanguageCssTest.exe LanguageTypeScriptTest.exe LineEndingsCRLFTest.exe
LineEndingsLFTest.exe QuickOpenEventuallyCompletesTest.exe
Regression1671Test.exe Regression2349EnewTest.exe
Regression2988SwitchEditorTest.exe Regression3084CommandLoopTest.exe
RegressionCommandLineNoCompletionTest.exe
RegressionFileModifiedIndicationTest.exe RegressionFontFallbackTest.exe
RegressionVspEmptyInitialBufferTest.exe RegressionNonExistentDirectory.exe
Expand Down
Loading

0 comments on commit 9982221

Please sign in to comment.