Fix indentation with OnTypeFormatting method for \n
#2922
Conversation
| new RequestType('ark/internal/onTypeFormatting'); | ||
|
|
||
| export function registerOnTypeFormatter(client: LanguageClient): Disposable { | ||
| const rSel = { scheme: 'file', language: 'r' } as vscode.DocumentSelector; |
There was a problem hiding this comment.
Should you be using these document selections instead?
positron/extensions/positron-r/src/lsp.ts
Lines 97 to 101 in 209b6c0
i.e. does it work in the console / do we want it to? does it work in untitled files?
(we should probably try and put that list of document selectors somewhere that can be shared between this and the LSP if we decide to use them)
There was a problem hiding this comment.
Ok this definitely doesn't work in untitled files so an update is definitely needed here!
There was a problem hiding this comment.
Good point! We now reuse the same selectors as the LSP.
This still doesn't work in Quarto documents, I've opened an issue: quarto-dev/quarto#432
There was a problem hiding this comment.
Somehow it doesn't work in the console. Would be nice if it did but I can't see this ever being a priority? Well I guess it depends on the workflow, I'm always surprised when I see folks entering multi-line code in the console, but I do see it sometimes (mostly python folks?). I tend to always write non-trivial code in a script and evaluate from there.
There was a problem hiding this comment.
Good hunch, unfortunately that didn't work.
Adds an LSP method for
OnTypeFormattingto correct the indentation of the frontend.Addresses #1880
Addresses #2764
Addresses #2707
See companion PR posit-dev/ark#329 (to be merged first) for details.
Intent
Fix remaining indentation issues that we haven't been able to fix due to the limitations of the regexp-based indentation rules of VS Code.
Approach
This PR enables the
formatOnTypefeature in the positron-r extension to let Ark correct problematic indentation based on the up-to-date tree-sitter representation of the documents. When the user hits enter, an LSP request is sent to format around\n. If the ideal indentation differs from what we managed to do with regular expression, Ark sends a correction.The correction is noticeable, i.e. you can see the cursor move more or less quickly. I haven't find this too distracting in my limited usage. This is also the approach taken for the Python extension with the proprietary pylance server providing indent corrections, and it's been working sufficiently well for them. If that is not sufficient for us, one potential approach would be to move the formatting engine to the renderer process, perhaps in the form of a wasm plugin, but that approach has lots of unknowns.
To work around the ordering issue of our LSP server that we rencently discovered (see #2692 (comment)), the LSP request is sent via a custom
OnTypeFormattingprovider for\nthat sends a custom formatting request to Ark. This allows sending a version ID for the document to detect an ordering problem. In that case we give up on the indent correction to prevent corruption of the document on the frontend side.The integration tests have been updated to record the correction in snapshots if there is any. This required adding a
waitLsp()method to synchronise the frontend and backend. In addition we're polling for runtime discovery to finish. It would be nice to have some better way of waiting for this in the future.QA notes
The indentation bugs in the linked issues should be fixed at top-level as well as in nested code, e.g. inside functions or other constructs.