Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Config to define files to be treated as JSON Schema files #37

Conversation

diyaayay
Copy link
Collaborator

Resolves #5

Changes:
-The user can now define the files to be watched, the default behavior is to watch all files.

Tested in both VSCode and Neovim.

image

WhatsApp Image 2024-05-11 at 01 44 33_cae6d68c

Copy link
Collaborator

@jdesrosiers jdesrosiers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't tried yet, but it seem like the initial workspace validation would always use the default patterns. Then during that initial workspace validation, getDocumentSettings happens to get called and sets schemaFilePatterns to the intended value. Then only the next time there's a change, would the settings would be used. Even if it is somehow getting set properly before first evaluation, we don't want to rely on that side effect.

Another thing that occurred to me is that this is a workspace level config, but the settings are essentially per document. It wouldn't make sense for one document to have a different schemaFilePatterns than another. I believe that vscode has a concept of different levels of configs including one on the workspace level and another on the document level. Does anything like that exist on the language server level? I think not, but it's worth checking.

Comment on lines +42 to +49
const matchers = schemaFilePatterns.map((pattern) => picomatch(
pattern,
{ globstar: true,
matchBase: true,
dot: true,
nonegate: true }

));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The following is my preferred code style for cases like this. I'm surprised the eslint config didn't catch the object literal style and the empty line.

Suggested change
const matchers = schemaFilePatterns.map((pattern) => picomatch(
pattern,
{ globstar: true,
matchBase: true,
dot: true,
nonegate: true }
));
const matchers = schemaFilePatterns.map((pattern) => {
return picomatch(pattern, {
globstar: true,
matchBase: true,
dot: true,
nonegate: true
});
});



setShouldValidateSchema(false);

let schemaFilePatterns = ["**/*.schema.json", "**/schema.json"];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to fold this into configuration? I think we can put this into globalSettings. Then we should be getting the value only from configuration.

@diyaayay
Copy link
Collaborator Author

I haven't tried yet, but it seem like the initial workspace validation would always use the default patterns. Then during that initial workspace validation, getDocumentSettings happens to get called and sets schemaFilePatterns to the intended value. Then only the next time there's a change, would the settings would be used. Even if it is somehow getting set properly before first evaluation, we don't want to rely on that side effect.

Another thing that occurred to me is that this is a workspace level config, but the settings are essentially per document. It wouldn't make sense for one document to have a different schemaFilePatterns than another. I believe that vscode has a concept of different levels of configs including one on the workspace level and another on the document level. Does anything like that exist on the language server level? I think not, but it's worth checking.

Ah yes! I think we need to have the user-defined patterns before the initial validation and that should sort this out. The different level of configs on the language server level is something I've been looking into. I did read into multi-root workspaces with config levels. I'll fix the initial validation bug and then check the workspace level and document level configs.

@diyaayay diyaayay closed this May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Server - Add a config to define what files should be treated as JSON Schema files
2 participants