feat: Diagnostics suppression for files with no pgm_conf configuration #89
Conversation
…-lsp-for-hlasm into suppress-diags # Conflicts: # language_server/test/dap/feature_launch_test.cpp
| { | ||
| if (suppress_diags_limit_) | ||
| return suppress_diags_limit_.value(); | ||
| return lib_config::get_instance()->diag_supress_limit; |
There was a problem hiding this comment.
shouldn't the lib_config instance be just another member of the workspace?
There was a problem hiding this comment.
The configuration can be changed by the user during lifetime of workspace, so we need to get the instance every time.
There was a problem hiding this comment.
I was thinking that perhaps each workspace might have different limits?
There was a problem hiding this comment.
Yes, right now the static lib_config instance represents the configuration of VS Code client. At the same time, you can override the limit of each workspace by writing the configuration into pgm_conf.json.
There was a problem hiding this comment.
I can get rid of the static instance and create a member instance of workspace_manager, would that be preferable?
But we still need some code to merge the global and workspace configurations.
There was a problem hiding this comment.
I think that you should not have a static instance at all.
There was a problem hiding this comment.
Sorry for late editing of the previous comment.
I meant adding the instance as a non-static member of workspace manager.
| @@ -114,7 +115,7 @@ class PARSER_LIBRARY_EXPORT workspace_manager | |||
| virtual void register_highlighting_consumer(highlighting_consumer* consumer); | |||
| virtual void register_diagnostics_consumer(diagnostics_consumer* consumer); | |||
| virtual void register_performance_metrics_consumer(performance_metrics_consumer* consumer); | |||
|
|
|||
| virtual void set_message_consumer(message_consumer* consumer); | |||
There was a problem hiding this comment.
can we unify the naming of consumer setting/registering?
There was a problem hiding this comment.
or, does it make sense to register more message consumers?
| #include "json.hpp" | ||
|
|
||
| #include "parser_library_export.h" | ||
|
|
There was a problem hiding this comment.
seems like this is important part of added logic. please add a comment describing this structure
parser_library/src/lib_config.cpp
Outdated
|
|
||
| #include "lib_config.h" | ||
|
|
||
| static std::shared_ptr<lib_config> config_instance = std::make_shared<lib_config>(); |
There was a problem hiding this comment.
could be anonymous namespace instead...
But I am still wondering if this should be part of the server's state.
|
|
||
|
|
||
| // if there is no processor group assigned to the program, delete diagnostics that may have been created | ||
| if (cancel_ && cancel_->load()) // skip, if parsing was cancelled using the cancellation token |
There was a problem hiding this comment.
can you simplify?
| if (cancel_ && cancel_->load()) // skip, if parsing was cancelled using the cancellation token | |
| if (cancel_ && *cancel_) // skip, if parsing was cancelled using the cancellation token |
|
Kudos, SonarCloud Quality Gate passed!
|
| @@ -58,6 +58,7 @@ class server : public response_provider | |||
| std::vector<std::unique_ptr<feature>> features_; | |||
|
|
|||
| std::map<std::string, method> methods_; | |||
| std::unordered_map<json, method> request_handlers_; | |||
There was a problem hiding this comment.
could you elaborate on why use json as the key?
# Conflicts: # language_server/src/lsp/feature_workspace_folders.cpp # language_server/test/lsp/lsp_server_test.cpp # language_server/test/response_provider_mock.h # parser_library/src/workspace_manager.cpp # parser_library/src/workspace_manager_impl.h
|
Kudos, SonarCloud Quality Gate passed! |
|
🎉 This PR is included in version 0.12.0-beta.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 0.12.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Added the diagnosticsSuppressLimit option, that a number and limits the number of diagnostics that are shown when there is no configuration for currently opened file in the pgm_conf. The reason why to introduce such limit is that the majority of HLASM source files use macros and copy files. When there is no configuration, symbols defined in those external files are not accessible thus the plugin shows many diagnostics that are not useful. To increase usability of the plugin in such cases, we do not show diagnostics at all.
The limit allows to edit small files with small amount of diagnostics even without configuration. It can be configured either in VS Code settings for user-specific configuration or in pgm_conf for project-specific configuration. When both are configured, the pgm_conf is prioritized.
workspace/didChangeConfigurationnotificationworkspace/configurationrequest