This PR fixes #49. It allows to configure if resources should be validated on save or on type (which is default one).
Changes
Introduced monokle.run with onType and onSave options (onType by default).
Reworked how extension watches for file changes. Before it was generic (provided by VSCode API) watcher and now dedicated events are used - workspace.onDidChangeTextDocument (onType), workspace.onDidSaveTextDocument (onSave), workspace.onDidCreateFiles and workspace.onDidDeleteFiles (for both). The important difference is that, generic watcher also reacted when files were changed by 3rd-party processes, while those listeners reacts only for changes triggered by VSC (or it's API) - still I think it's reasonable in context of better efficiency we get from dedicated listeners.
Made use of MonokleValidatorincremental flag - we know exactly which file changes (and it's dirty content if it's onType related) so incremental flag can be used in multiple cases.
There is important case to have in mind, incremental works by providing resourceId and for change events we get files. Each file can have multiple resources. And for example when one resource definition is removed from such file we can't use incremental (because we want to tell validator to revalidate removed resource which id we don't have). In such cases incremental is not used.
What was tricky (or I just didn't anticipated it π ), is that for onType we need to get dirty file content (as it is not saved yet) and pass it to validator - before, we always read file content for validation from disk.
From code perspective it wasn't such a big rework but conceptually it was, and I think it's not fully there with this solution (see remarks below).
I'm not fully happy with the solution. From functional and performance-related perspective I think it's really good improvement. For example, before, for every file saved, entire workspace directory was read to get files via globing **/**.yaml (even to get single file content ππ), so now with dedicated listeners it's much better.
But from the code and architecture perspective after spending some time with this and all the edge cases - where you need to validate dirty files, keeping track of resources in the file to determine if use incremental validation, local config can get dirty, etc. it become complex in the existing form.
I think the better approach would be keeping and abstract "living" file list with all the manifests and their content which reacts on file changes and stores most recent content (dirty or read from fs) and then always validate using this abstraction instead of directly reading from fs when validating. But it will be another 2-3 days of work and since I realized this a bit too early, didn't want to spend more time on refactoring working solution.
This PR fixes #49. It allows to configure if resources should be validated on save or on type (which is default one).
Changes
monokle.run
withonType
andonSave
options (onType
by default).workspace.onDidChangeTextDocument
(onType),workspace.onDidSaveTextDocument
(onSave),workspace.onDidCreateFiles
andworkspace.onDidDeleteFiles
(for both). The important difference is that, generic watcher also reacted when files were changed by 3rd-party processes, while those listeners reacts only for changes triggered by VSC (or it's API) - still I think it's reasonable in context of better efficiency we get from dedicated listeners.MonokleValidator
incremental
flag - we know exactly which file changes (and it's dirty content if it's onType related) so incremental flag can be used in multiple cases.resourceId
and for change events we get files. Each file can have multiple resources. And for example when one resource definition is removed from such file we can't use incremental (because we want to tell validator to revalidate removed resource which id we don't have). In such cases incremental is not used.onType
we need to get dirty file content (as it is not saved yet) and pass it to validator - before, we always read file content for validation from disk.See API docs for reference - https://code.visualstudio.com/api/references/vscode-api.
Fixes
Remarks
I'm not fully happy with the solution. From functional and performance-related perspective I think it's really good improvement. For example, before, for every file saved, entire workspace directory was read to get files via globing
**/**.yaml
(even to get single file content ππ), so now with dedicated listeners it's much better. But from the code and architecture perspective after spending some time with this and all the edge cases - where you need to validate dirty files, keeping track of resources in the file to determine if use incremental validation, local config can get dirty, etc. it become complex in the existing form. I think the better approach would be keeping and abstract "living" file list with all the manifests and their content which reacts on file changes and stores most recent content (dirty or read from fs) and then always validate using this abstraction instead of directly reading from fs when validating. But it will be another 2-3 days of work and since I realized this a bit too early, didn't want to spend more time on refactoring working solution.Checklist