microsoft / vscode-cpptools

Official repository for the Microsoft C/C++ extension for VS Code.
Other
5.52k stars 1.55k forks source link

Formatting can be delayed by slow IntelliSense operations #6156

Open bobbrow opened 4 years ago

bobbrow commented 4 years ago

Thanks! Unfortunately, the experience here is a bit janky for me -- there's a substantial pause between the time when I insert a newline, and when indentation is applied. It seems like sometimes the indentation attempt just ... times out? It's worth saying that I'm editing a file with a substantial number of large header includes (e.g. Boost); not sure if that effects things here.

vscode-indent-slow

If I'm typing quickly, then no attempt at indentation happens at all (and also notice a rogue right parenthesis gets duplicated; normally ) insertion would just move over the already-inserted paren):

vscode-indent-slow-2

All that said -- rather than a "smart-enough post-hoc indentation system", I'm really hoping that cpptools could just immediately place the cursor in the "right" place after a newline is inserted. (I imagine this is incredibly challenging to solve in general due to the complexity of the C++ grammar, but I think at least in this case it should be easy?)

It seems like a lot of C++ developers using VSCode are used to this workflow (write code without as-you-type indentation, and then format afterwords using an appropriate formatter) but coming from other editing environments (e.g. Vim) I'm used to seeing the cursor just "be in the right place" with newline indentation.

Originally posted by @kevinushey in https://github.com/microsoft/vscode-cpptools/issues/883#issuecomment-693571844

bobbrow commented 4 years ago

Adding @Colengms to help look into this. As I said in the other issue, the formatter is purely lexical and doesn't require processing included headers, so it shouldn't be related to your usage of boost. If you can enable Debug logging and share the output that appears when you try this, I think that would be a good starting point for the investigation.

kevinushey commented 4 years ago

Thanks! I think the debug log makes it clear what's happening here -- the language server is stalling while attempting to serve a completion request (triggered while I was typing), and then the indentation request only goes through after the completion request is canceled. Here is the log:

textDocument/didChange: file:///Users/kevinushey/rstudio-v14/src/cpp/session/modules/SessionSource.cpp
cpptools/textEditorSelectionChange
textDocument/completion: file:///Users/kevinushey/rstudio-v14/src/cpp/session/modules/SessionSource.cpp (id: 209)
auto_complete::handle_completion: file:///Users/kevinushey/rstudio-v14/src/cpp/session/modules/SessionSource.cpp (60:2)
Offering completion   # <-- pause here
sending 1 changes to server
sending 1 changes to server
sending 1 changes to server
Request canceled: 209
cpptools/getFoldingRanges: file:///Users/kevinushey/rstudio-v14/src/cpp/session/modules/SessionSource.cpp (id: 211)
textDocument/didChange: file:///Users/kevinushey/rstudio-v14/src/cpp/session/modules/SessionSource.cpp
cpptools/abortRequest
cpptools/textEditorSelectionChange
textDocument/didChange: file:///Users/kevinushey/rstudio-v14/src/cpp/session/modules/SessionSource.cpp
cpptools/textEditorSelectionChange
textDocument/didChange: file:///Users/kevinushey/rstudio-v14/src/cpp/session/modules/SessionSource.cpp
cpptools/textEditorSelectionChange
textDocument/didChange: file:///Users/kevinushey/rstudio-v14/src/cpp/session/modules/SessionSource.cpp
cpptools/textEditorSelectionChange
textDocument/didChange: file:///Users/kevinushey/rstudio-v14/src/cpp/session/modules/SessionSource.cpp
cpptools/textEditorSelectionChange
textDocument/didChange: file:///Users/kevinushey/rstudio-v14/src/cpp/session/modules/SessionSource.cpp
cpptools/textEditorSelectionChange
textDocument/didChange: file:///Users/kevinushey/rstudio-v14/src/cpp/session/modules/SessionSource.cpp
cpptools/textEditorSelectionChange
textDocument/didChange: file:///Users/kevinushey/rstudio-v14/src/cpp/session/modules/SessionSource.cpp
cpptools/textEditorSelectionChange
textDocument/didChange: file:///Users/kevinushey/rstudio-v14/src/cpp/session/modules/SessionSource.cpp
cpptools/textEditorSelectionChange
textDocument/signatureHelp: file:///Users/kevinushey/rstudio-v14/src/cpp/session/modules/SessionSource.cpp (id: 219)
Request canceled: 219
cpptools/getFoldingRanges: file:///Users/kevinushey/rstudio-v14/src/cpp/session/modules/SessionSource.cpp (id: 220)
textDocument/didChange: file:///Users/kevinushey/rstudio-v14/src/cpp/session/modules/SessionSource.cpp
cpptools/abortRequest
cpptools/textEditorSelectionChange
textDocument/signatureHelp: file:///Users/kevinushey/rstudio-v14/src/cpp/session/modules/SessionSource.cpp (id: 221)
Request canceled: 221
textDocument/didChange: file:///Users/kevinushey/rstudio-v14/src/cpp/session/modules/SessionSource.cpp
cpptools/textEditorSelectionChange
textDocument/didChange: file:///Users/kevinushey/rstudio-v14/src/cpp/session/modules/SessionSource.cpp
cpptools/textEditorSelectionChange
textDocument/didChange: file:///Users/kevinushey/rstudio-v14/src/cpp/session/modules/SessionSource.cpp
cpptools/textEditorSelectionChange
textDocument/didChange: file:///Users/kevinushey/rstudio-v14/src/cpp/session/modules/SessionSource.cpp
cpptools/textEditorSelectionChange
textDocument/documentHighlight: file:///Users/kevinushey/rstudio-v14/src/cpp/session/modules/SessionSource.cpp (id: 225)
Request canceled: 225
textDocument/didChange: file:///Users/kevinushey/rstudio-v14/src/cpp/session/modules/SessionSource.cpp
cpptools/textEditorSelectionChange
textDocument/signatureHelp: file:///Users/kevinushey/rstudio-v14/src/cpp/session/modules/SessionSource.cpp (id: 226)
Request canceled: 226
textDocument/didChange: file:///Users/kevinushey/rstudio-v14/src/cpp/session/modules/SessionSource.cpp
cpptools/textEditorSelectionChange
cpptools/formatOnType: file:///Users/kevinushey/rstudio-v14/src/cpp/session/modules/SessionSource.cpp (id: 227)
Formatting input:
/*
 * SessionSource.cpp
 *
 * Copyright (C) 2020 by RStudio, PBC
 *
 * Unless you have received this program directly from RStudio pursuant
 * to the terms of a commercial license agreement with RStudio, then
 * this program is licensed to you under the terms of version 3 of the
 * GNU Affero General Public License. This program is distributed WITHOUT
 * ANY EXPRESS OR IMPLIED WARRANTY, INCLUDING THOSE OF NON-INFRINGEMENT,
 * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE. Please refer to the
 * AGPL (http://www.gnu.org/licenses/agpl-3.0.txt) for more details.
 *
 */

#include "SessionSource.hpp"
#include "rmarkdown/NotebookChunkDefs.hpp"

#include <string>
#include <map>
#include <fstream>

#include <gsl/gsl>

#include <boost/bind.hpp>
#include <boost/utility.hpp>

#include <core/r_util/RSourceIndex.hpp>

#include <core/Log.hpp>
#include <core/Exec.hpp>
#include <shared_core/Error.hpp>
#include <shared_core/FilePath.hpp>
#include <core/FileInfo.hpp>
#include <core/FileSerializer.hpp>
#include <core/...
Formatting document: file:///Users/kevinushey/rstudio-v14/src/cpp/session/modules/SessionSource.cpp
Formatting Engine: vcFormat
textDocument/signatureHelp: file:///Users/kevinushey/rstudio-v14/src/cpp/session/modules/SessionSource.cpp (id: 228)
sending 1 changes to server
sending 1 changes to server
sending 1 changes to server
sending 1 changes to server
sending 1 changes to server
sending 1 changes to server
sending 1 changes to server
sending 1 changes to server
sending 1 changes to server
sending 1 changes to server
sending 1 changes to server
sending 1 changes to server
sending 1 changes to server
sending 1 changes to server
sending 1 changes to server
sending 1 changes to server
Checking for syntax errors: file:///Users/kevinushey/rstudio-v14/src/cpp/session/modules/SessionSource.cpp
Queueing IntelliSense update for files in translation unit of: /Users/kevinushey/rstudio-v14/src/cpp/session/modules/SessionSource.cpp
  tag parsing file: /Users/kevinushey/rstudio-v14/src/cpp/session/modules/SessionSource.cpp
Request canceled: 228
cpptools/getFoldingRanges: file:///Users/kevinushey/rstudio-v14/src/cpp/session/modules/SessionSource.cpp (id: 229)
cpptools/getCodeActions: file:///Users/kevinushey/rstudio-v14/src/cpp/session/modules/SessionSource.cpp (id: 230)
textDocument/didChange: file:///Users/kevinushey/rstudio-v14/src/cpp/session/modules/SessionSource.cpp
cpptools/textEditorSelectionChange
cpptools/abortRequest
textDocument/signatureHelp: file:///Users/kevinushey/rstudio-v14/src/cpp/session/modules/SessionSource.cpp (id: 231)
sending 1 changes to server
cpptools/getFoldingRanges: file:///Users/kevinushey/rstudio-v14/src/cpp/session/modules/SessionSource.cpp (id: 232)
cpptools/getCodeActions: file:///Users/kevinushey/rstudio-v14/src/cpp/session/modules/SessionSource.cpp (id: 233)
cpptools/finishUpdateSquiggles
Error squiggle count: 133
Update IntelliSense time (sec): 0.272
cpptools/getDocumentSymbols: file:///Users/kevinushey/rstudio-v14/src/cpp/session/modules/SessionSource.cpp (id: 234)
cpptools/getDocumentSymbols
cpptools/getCodeActions: file:///Users/kevinushey/rstudio-v14/src/cpp/session/modules/SessionSource.cpp (id: 235)
cpptools/getSemanticTokens: file:///Users/kevinushey/rstudio-v14/src/cpp/session/modules/SessionSource.cpp (id: 236)
Checking for syntax errors: file:///Users/kevinushey/rstudio-v14/src/cpp/session/modules/SessionSource.cpp
Queueing IntelliSense update for files in translation unit of: /Users/kevinushey/rstudio-v14/src/cpp/session/modules/SessionSource.cpp
  tag parsing file: /Users/kevinushey/rstudio-v14/src/cpp/session/modules/SessionSource.cpp
cpptools/finishUpdateSquiggles
Error squiggle count: 3
Update IntelliSense time (sec): 0.257
cpptools/getSemanticTokens: file:///Users/kevinushey/rstudio-v14/src/cpp/session/modules/SessionSource.cpp (id: 237)
cpptools/getCodeActions: file:///Users/kevinushey/rstudio-v14/src/cpp/session/modules/SessionSource.cpp (id: 238)
Database safe to open
Colengms commented 4 years ago

There are a few possible issues here. It's possible another IntelliSense operation was occurring, which can cause subsequent requests to our message handler (such as format on-type requests) to be delayed. @kevinushey , can you provide the output of the C/C++ channel while this repro's, perhaps using a similar animated image, so we can see the timing? (The log you just posted as I was typing this seems to confirm that hypothesis). Depending on the complexity of the code-base, IntelliSense requests that could block on-type formatting could take a significantly long time.

Another problem is that asynchronous on-type formatting is fundamentally flawed, IMHO. It only works if we can always service it immediately, before the user types the next character. If formatting takes longer than a single keypress to process, the results are discarded. An issue here can be repro'ed simply by holding down a key that triggers formatting (i.e. enter, with indent rules enabled). Ideally, formatting should not be asynchronous, but all interactions between VS Code and extensions are asynchronous with regard to user input. I suspect we may need to remove vcFormat's indent on-type features.

Colengms commented 4 years ago

Regarding the extra closing paren: That seems to be a VS Code issue. I can repro with formatting disabled, by pressing enter between typing params, before typing the closing paren.

kevinushey commented 4 years ago

Here's a live example with output:

vscode-indent

kevinushey commented 4 years ago

Another problem is that asynchronous on-type formatting is fundamentally flawed, IMHO. It only works if we can always service it immediately, before the user types the next character. If formatting takes longer than a single keypress to process, the results are discarded. An issue here can be repro'ed simply by holding down a key that triggers formatting (i.e. enter, with indent rules enabled). Ideally, formatting should not be asynchronous, but all interactions between VS Code and extensions are asynchronous with regard to user input. I suspect we may need to remove vcFormat's indent on-type features.

I would overall agree with this. Rather than relying on formatters to perform as-you-type indentation, I think that indentation should be handled by a custom command (bound to the Return key) that makes "best guesses" as to what the right indentation should be. Such an indentation system could either rely on regular expressions, or an ad-hoc walk of a simple tokenized representation of the document (basically just doing bracket counting or looking for "special" tokens to guess what the right indent is).

(For what it's worth, this is basically what the python indent extension does, except it relies on the python-indent-parser module for the heavy lifting)