Introduce a limit to outline tree size #194
Conversation
|
Kudos, SonarCloud Quality Gate passed! |
| @@ -320,7 +320,8 @@ void feature_language_features::document_symbol(const json& id, const json& para | |||
| { | |||
| auto document_uri = params["textDocument"]["uri"].get<std::string>(); | |||
|
|
|||
| auto symbol_list = ws_mngr_.document_symbol(uri_to_path(document_uri).c_str()); | |||
| const auto limit = 5000LL; | |||
| auto symbol_list = ws_mngr_.document_symbol(uri_to_path(document_uri).c_str(), limit); | |||
There was a problem hiding this comment.
Would it be better to make the limit configurable by adding it to the user configuration and then using the configuration changed notification (which is already implemented). Might be even easier, since no edits to workspace_manager would be required.
There was a problem hiding this comment.
Regarding the user configurable option - given how poorly vscode behaves when too many nodes are reported, I think we would just end up giving users an option that could only cause problems.
There was a problem hiding this comment.
Regarding the workspace_manager changes, that could be done, but lsp_context and workspace share the interface, so if there is to be a parameter on the document_symbol function, it must be present on the workspace version as well.
There was a problem hiding this comment.
Maybe I would prefer that workspace would not implement the interface and put the limit into lib_config. But then lib_config wouldn't represent only user-configurable options.
I guess I am fine with both versions, then.
|
🎉 This PR is included in version 0.15.0-beta.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 0.15.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
No description provided.