Skip to content

Commit 452ef92

Browse files
bryphebryphe
authored
feat(formatting/#2464): Initial format-on-save implementation (#3246)
This implements format-on-save, adding an `"editor.formatOnSave"` configuration setting that can be set per-filetype, and then actually implementing the format-on-save behavior. __TODO:__ - [x] Fix up notification message (when applying edits due to save, update to say something like "Saved with X formatting edits from <format-provider>") - [x] Move the write command to `Feature_Vim.Effects.save(~bufferId)` - [x] Investigate adjusting cursor position in response to formatting edits (#3263) - [x] Investigate limits for large files - [x] Add test case Fixes #2464
1 parent dfa48a1 commit 452ef92

12 files changed

+475
-109
lines changed

‎.prettierignore

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
integration_test/*

‎CHANGES_CURRENT.md

+1
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
### Features
1010

1111
- #3163 - UX: Add clear notification button (thanks @andr3h3nriqu3s11 !)
12+
- #3246 - Formatting: Add 'editor.formatOnSave' configuration (fixes #2464)
1213

1314
### Bug Fixes
1415

‎integration_test/FormatOnSaveTest.re

+96
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
open Oni_Core;
2+
open Oni_Model;
3+
open Oni_IntegrationTestLib;
4+
5+
module TS = TextSynchronization;
6+
7+
let configuration = {|
8+
{
9+
"editor.formatOnSave": true
10+
}
11+
|};
12+
13+
runTest(
14+
~configuration=Some(configuration),
15+
~name="FormatOnSave",
16+
({input, dispatch, wait, key, _}) => {
17+
wait(~timeout=30.0, ~name="Exthost is initialized", (state: State.t) =>
18+
Feature_Exthost.isInitialized(state.exthost)
19+
);
20+
21+
// Wait until the extension is activated
22+
// Give some time for the exthost to start
23+
wait(
24+
~timeout=30.0,
25+
~name="Validate the 'oni-dev' extension gets activated",
26+
(state: State.t) =>
27+
List.exists(
28+
id => id == "oni-dev-extension",
29+
state.extensions |> Feature_Extensions.activatedIds,
30+
)
31+
);
32+
33+
let unformattedFilePath = getAssetPath("test.unformatted.js");
34+
35+
// Create a buffer
36+
dispatch(Actions.OpenFileByPath(unformattedFilePath, None, None));
37+
38+
wait(
39+
~timeout=30.0,
40+
~name="Validate first line of buffer is unformatted",
41+
(state: State.t) => {
42+
Selectors.getActiveBuffer(state)
43+
|> Option.map(buffer =>
44+
if (Buffer.getNumberOfLines(buffer) > 0) {
45+
let rawLine = Buffer.getLine(0, buffer) |> BufferLine.raw;
46+
47+
String.equal(rawLine, " console.log");
48+
} else {
49+
false;
50+
}
51+
)
52+
|> Option.value(~default=false)
53+
});
54+
55+
// Wait for JS extension, we need it for formatting
56+
wait(
57+
~timeout=30.0,
58+
~name=
59+
"Validate the 'vscode.typescript-language-features' extension gets activated",
60+
(state: State.t) => {
61+
Feature_Extensions.activatedIds(state.extensions)
62+
|> List.iter(id => prerr_endline(id));
63+
List.exists(
64+
id => id == "vscode.typescript-language-features",
65+
state.extensions |> Feature_Extensions.activatedIds,
66+
);
67+
},
68+
);
69+
70+
// Save file
71+
input(":");
72+
input("w");
73+
input("!");
74+
key(EditorInput.Key.Return);
75+
76+
wait(
77+
~timeout=30.0,
78+
~name="Validate first line of buffer gets formatted",
79+
(state: State.t) => {
80+
Selectors.getActiveBuffer(state)
81+
|> Option.map(buffer => {
82+
let rawLine = Buffer.getLine(0, buffer) |> BufferLine.raw;
83+
84+
String.equal(rawLine, "console.log");
85+
})
86+
|> Option.value(~default=false)
87+
});
88+
89+
TS.validateTextIsSynchronized(
90+
~expectedText=Some("console.log|console.warn|console.error|"),
91+
~description="after formatting",
92+
dispatch,
93+
wait,
94+
);
95+
},
96+
);

‎integration_test/dune

+22-22
Original file line numberDiff line numberDiff line change
@@ -9,24 +9,24 @@
99
ExCommandKeybindingNormTest ExtConfigurationChangedTest
1010
ExtHostBufferOpenTest ExtHostBufferUpdatesTest
1111
ExtHostCommandActivationTest ExtHostCompletionTest ExtHostDefinitionTest
12-
ExtHostWorkspaceSearchTest FileWatcherTest KeybindingsInvalidJsonTest
13-
KeySequenceJJTest InputIgnoreTest InputContextKeysTest
14-
InputRemapMotionTest LanguageCssTest LanguageTypeScriptTest
15-
LineEndingsLFTest LineEndingsCRLFTest QuickOpenEventuallyCompletesTest
16-
SearchShowClearHighlightsTest SyntaxServer SyntaxServerCloseTest
17-
SyntaxServerMessageExceptionTest SyntaxServerParentPidTest
18-
SyntaxServerParentPidCorrectTest SyntaxServerReadExceptionTest
19-
Regression1671Test Regression2349EnewTest Regression2988SwitchEditorTest
20-
Regression3084CommandLoopTest RegressionCommandLineNoCompletionTest
21-
RegressionFontFallbackTest RegressionFileModifiedIndicationTest
22-
RegressionNonExistentDirectory RegressionVspEmptyInitialBufferTest
23-
RegressionVspEmptyExistingBufferTest SCMGitTest SyntaxHighlightPhpTest
24-
SyntaxHighlightTextMateTest SyntaxHighlightTreesitterTest
25-
AddRemoveSplitTest TerminalSetPidTitleTest TerminalConfigurationTest
26-
TypingBatchedTest TypingUnbatchedTest VimIncsearchScrollTest
27-
VimExperimentalVimlTest VimRemapTimeoutTest VimSimpleRemapTest
28-
VimlRemapCmdlineTest ClipboardChangeTest VimScriptLocalFunctionTest
29-
ZenModeSingleFileModeTest ZenModeSplitTest)
12+
ExtHostWorkspaceSearchTest FileWatcherTest FormatOnSaveTest
13+
KeybindingsInvalidJsonTest KeySequenceJJTest InputIgnoreTest
14+
InputContextKeysTest InputRemapMotionTest LanguageCssTest
15+
LanguageTypeScriptTest LineEndingsLFTest LineEndingsCRLFTest
16+
QuickOpenEventuallyCompletesTest SearchShowClearHighlightsTest
17+
SyntaxServer SyntaxServerCloseTest SyntaxServerMessageExceptionTest
18+
SyntaxServerParentPidTest SyntaxServerParentPidCorrectTest
19+
SyntaxServerReadExceptionTest Regression1671Test Regression2349EnewTest
20+
Regression2988SwitchEditorTest Regression3084CommandLoopTest
21+
RegressionCommandLineNoCompletionTest RegressionFontFallbackTest
22+
RegressionFileModifiedIndicationTest RegressionNonExistentDirectory
23+
RegressionVspEmptyInitialBufferTest RegressionVspEmptyExistingBufferTest
24+
SCMGitTest SyntaxHighlightPhpTest SyntaxHighlightTextMateTest
25+
SyntaxHighlightTreesitterTest AddRemoveSplitTest TerminalSetPidTitleTest
26+
TerminalConfigurationTest TypingBatchedTest TypingUnbatchedTest
27+
VimIncsearchScrollTest VimExperimentalVimlTest VimRemapTimeoutTest
28+
VimSimpleRemapTest VimlRemapCmdlineTest ClipboardChangeTest
29+
VimScriptLocalFunctionTest ZenModeSingleFileModeTest ZenModeSplitTest)
3030
(libraries Oni_CLI Oni_IntegrationTestLib reason-native-crash-utils.asan))
3131

3232
(install
@@ -45,7 +45,7 @@
4545
ExtConfigurationChangedTest.exe ExtHostBufferOpenTest.exe
4646
ExtHostBufferUpdatesTest.exe ExtHostCommandActivationTest.exe
4747
ExtHostCompletionTest.exe ExtHostDefinitionTest.exe
48-
ExtHostWorkspaceSearchTest.exe FileWatcherTest.exe
48+
ExtHostWorkspaceSearchTest.exe FileWatcherTest.exe FormatOnSaveTest.exe
4949
KeybindingsInvalidJsonTest.exe KeySequenceJJTest.exe InputIgnoreTest.exe
5050
InputContextKeysTest.exe InputRemapMotionTest.exe LanguageCssTest.exe
5151
LanguageTypeScriptTest.exe LineEndingsCRLFTest.exe LineEndingsLFTest.exe
@@ -67,6 +67,6 @@
6767
VimScriptLocalFunctionTest.exe VimRemapTimeoutTest.exe
6868
VimSimpleRemapTest.exe VimlRemapCmdlineTest.exe
6969
ZenModeSingleFileModeTest.exe ZenModeSplitTest.exe ClipboardChangeTest.exe
70-
large-c-file.c lsan.supp some-test-file.json some-test-file.txt test.crlf
71-
test.lf test-syntax.php utf8.txt utf8-test-file.htm
72-
Inconsolata-Regular.ttf PlugScriptLocal.vim))
70+
test.unformatted.js large-c-file.c lsan.supp some-test-file.json
71+
some-test-file.txt test.crlf test.lf test-syntax.php utf8.txt
72+
utf8-test-file.htm Inconsolata-Regular.ttf PlugScriptLocal.vim))

‎integration_test/test.unformatted.js

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
console.log
2+
console.warn
3+
console.error

‎src/Feature/LanguageSupport/Feature_LanguageSupport.re

+24-8
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,11 @@ type outmsg =
5454
insertText: string,
5555
additionalEdits: list(Exthost.Edit.SingleEditOperation.t),
5656
})
57+
| FormattingApplied({
58+
displayName: string,
59+
editCount: int,
60+
needsToSave: bool,
61+
})
5762
| InsertSnippet({
5863
meetColumn: CharacterIndex.t,
5964
snippet: string,
@@ -86,6 +91,8 @@ let map: ('a => msg, Outmsg.internalMsg('a)) => outmsg =
8691
fun
8792
| Outmsg.ApplyCompletion({meetColumn, insertText, additionalEdits}) =>
8893
ApplyCompletion({meetColumn, insertText, additionalEdits})
94+
| Outmsg.FormattingApplied({displayName, editCount, needsToSave}) =>
95+
FormattingApplied({displayName, editCount, needsToSave})
8996
| Outmsg.InsertSnippet({meetColumn, snippet, additionalEdits}) =>
9097
InsertSnippet({meetColumn, snippet, additionalEdits})
9198
| Outmsg.Nothing => Nothing
@@ -382,14 +389,8 @@ let update =
382389
| Formatting.Nothing => Nothing
383390
| Formatting.Effect(eff) =>
384391
Effect(eff |> Isolinear.Effect.map(msg => Formatting(msg)))
385-
| Formatting.FormattingApplied({displayName, editCount}) =>
386-
NotifySuccess(
387-
Printf.sprintf(
388-
"Formatting: Applied %d edits with %s",
389-
editCount,
390-
displayName,
391-
),
392-
)
392+
| Formatting.FormattingApplied({displayName, editCount, needsToSave}) =>
393+
FormattingApplied({displayName, editCount, needsToSave})
393394
| Formatting.FormatError(errorMsg) => NotifyFailure(errorMsg)
394395
| Formatting.ShowMenu(menu) =>
395396
let menu' =
@@ -486,6 +487,21 @@ let bufferUpdated =
486487
{...model, completion, signatureHelp};
487488
};
488489

490+
let bufferSaved = (~isLargeBuffer, ~buffer, ~config, ~activeBufferId, model) => {
491+
let (formatting', formattingEffect) =
492+
Formatting.bufferSaved(
493+
~isLargeBuffer,
494+
~buffer,
495+
~config,
496+
~activeBufferId,
497+
model.formatting,
498+
);
499+
(
500+
{...model, formatting: formatting'},
501+
formattingEffect |> Isolinear.Effect.map(msg => Formatting(msg)),
502+
);
503+
};
504+
489505
let configurationChanged = (~config, model) => {
490506
...model,
491507
completion: Completion.configurationChanged(~config, model.completion),

‎src/Feature/LanguageSupport/Feature_LanguageSupport.rei

+15
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,11 @@ type outmsg =
5757
insertText: string,
5858
additionalEdits: list(Exthost.Edit.SingleEditOperation.t),
5959
})
60+
| FormattingApplied({
61+
displayName: string,
62+
editCount: int,
63+
needsToSave: bool,
64+
})
6065
| InsertSnippet({
6166
meetColumn: CharacterIndex.t,
6267
snippet: string,
@@ -99,6 +104,16 @@ let update:
99104
) =>
100105
(model, outmsg);
101106

107+
let bufferSaved:
108+
(
109+
~isLargeBuffer: bool,
110+
~buffer: Oni_Core.Buffer.t,
111+
~config: Oni_Core.Config.resolver,
112+
~activeBufferId: int,
113+
model
114+
) =>
115+
(model, Isolinear.Effect.t(msg));
116+
102117
let bufferUpdated:
103118
(
104119
~languageConfiguration: Oni_Core.LanguageConfiguration.t,

0 commit comments

Comments
 (0)