Skip to content

Commit

Permalink
fix(formatting/#3014): 'Invalid range specified' error (#3240)
Browse files Browse the repository at this point in the history
__Issue:__ When formatting a JavaScript file, or other file without a formatter installed by default, and formatting (either via the 'Format Document' command, or key sequences like `gg=G`) - there'd be a confusing 'Invalid range specified' error.

__Defect:__ There were actually several issues:
1) When falling back to the default format provider, there was an off-by-one bug that caused that error to be displayed when the whole document was requested to be formatted.
2) In the case of #3014 , there actually is a default format provider - the type script language feature provides a formatter, but it is a range formatter (not a document formatter). However, in the case where there is a range formatter but no document formatter, we can fall back to using the range formatter.
3) The case of multiple format providers wasn't handled in a robust way

__Fix:__
1) Fix the issues with the ranges in the default and language server range formatters
2) Implement logic to fall back to the range formatter in the case where there is no document formatter available
3) Implement proper resolution of format providers. When there are multiple, prompt the user to pick a default formatter. Use the `"editor.defaultFormatter": "publisher.extension-id"` configuration setting to disambiguate, for example:
```
"[javascript]": {
   "editor.defaultFormatter": "esbenp.prettier-vscode"
}
```
When the user selects a default, automatically update the configuration.
  • Loading branch information
bryphe authored Mar 6, 2021
1 parent e6c6705 commit 4431f88
Show file tree
Hide file tree
Showing 11 changed files with 286 additions and 51 deletions.
1 change: 1 addition & 0 deletions CHANGES_CURRENT.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
- #3230 - Font: Treat "FiraCode-Regular.ttf" as default font
- #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)

### Performance

Expand Down
10 changes: 10 additions & 0 deletions src/Core/ConfigurationTransformer.re
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,16 @@ let setField = (fieldName, value, json) => {
};
};

let setFiletypeField = (~fileType, fieldName, value, json) => {
json
|> Utility.JsonEx.update(
"[" ++ fileType ++ "]",
fun
| None => Some(`Assoc([(fieldName, value)]))
| Some(perFileJson) => Some(setField(fieldName, value, perFileJson)),
);
};

let removeField = (fieldName, json) => {
switch (json) {
| `Assoc(elems) =>
Expand Down
2 changes: 2 additions & 0 deletions src/Core/Utility/ListEx.re
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ let boundedLength = (~max, list) => {
loop(0, list);
};

let nth_opt = (idx, list) => List.nth_opt(list, idx);

/**
* Return the last element in a list.
*/
Expand Down
35 changes: 31 additions & 4 deletions src/Feature/LanguageSupport/Feature_LanguageSupport.re
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,9 @@ type outmsg =
| SetSelections({
editorId: int,
ranges: list(CharacterRange.t),
});
})
| ShowMenu(Feature_Quickmenu.Schema.menu(msg))
| TransformConfiguration(Oni_Core.ConfigurationTransformer.t);

let map: ('a => msg, Outmsg.internalMsg('a)) => outmsg =
f =>
Expand All @@ -101,7 +103,11 @@ let map: ('a => msg, Outmsg.internalMsg('a)) => outmsg =
}) =>
CodeLensesChanged({handle, bufferId, lenses, startLine, stopLine})
| Outmsg.SetSelections({editorId, ranges}) =>
SetSelections({editorId, ranges});
SetSelections({editorId, ranges})
| Outmsg.ShowMenu(menu) =>
ShowMenu(menu |> Feature_Quickmenu.Schema.map(f))
| Outmsg.TransformConfiguration(transformer) =>
TransformConfiguration(transformer);

module Msg = {
let exthost = msg => Exthost(msg);
Expand Down Expand Up @@ -215,24 +221,36 @@ let update =
({...model, completion: completion'}, Nothing);

| Exthost(
RegisterRangeFormattingSupport({handle, selector, displayName, _}),
RegisterRangeFormattingSupport({
handle,
selector,
displayName,
extensionId,
}),
) =>
let formatting' =
Formatting.registerRangeFormatter(
~handle,
~selector,
~extensionId,
~displayName,
model.formatting,
);
({...model, formatting: formatting'}, Nothing);

| Exthost(
RegisterDocumentFormattingSupport({handle, selector, displayName, _}),
RegisterDocumentFormattingSupport({
handle,
selector,
displayName,
extensionId,
}),
) =>
let formatting' =
Formatting.registerDocumentFormatter(
~handle,
~selector,
~extensionId,
~displayName,
model.formatting,
);
Expand Down Expand Up @@ -349,6 +367,7 @@ let update =
| Formatting(formatMsg) =>
let (formatting', outMsg) =
Formatting.update(
~config,
~languageConfiguration,
~maybeSelection,
~maybeBuffer,
Expand All @@ -372,6 +391,13 @@ let update =
),
)
| Formatting.FormatError(errorMsg) => NotifyFailure(errorMsg)
| Formatting.ShowMenu(menu) =>
let menu' =
menu |> Feature_Quickmenu.Schema.map(msg => Formatting(msg));
ShowMenu(menu');

| Formatting.TransformConfiguration(transformer) =>
TransformConfiguration(transformer)
};

({...model, formatting: formatting'}, outMsg');
Expand Down Expand Up @@ -600,6 +626,7 @@ module Contributions = {
CodeLens.Contributions.configuration
@ Completion.Contributions.configuration
@ DocumentHighlights.Contributions.configuration
@ Formatting.Contributions.configuration
@ SignatureHelp.Contributions.configuration;

let contextKeys =
Expand Down
4 changes: 3 additions & 1 deletion src/Feature/LanguageSupport/Feature_LanguageSupport.rei
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,9 @@ type outmsg =
| SetSelections({
editorId: int,
ranges: list(CharacterRange.t),
});
})
| ShowMenu(Feature_Quickmenu.Schema.menu(msg))
| TransformConfiguration(ConfigurationTransformer.t);

let update:
(
Expand Down
Loading

0 comments on commit 4431f88

Please sign in to comment.