Comments (10)
Another option would be adding support for checking the schema with a validator to give even better messages.
@remcohaszing contributed a few in SchemaStore/schemastore#1724 and SchemaStore/schemastore#1982 which could be leveraged
from unified-language-server.
This issue was particularly about missing unit tests. A comment in the code says something cannot occur, but it can, and should be tested.
Some examples: a plugin throwing an error, a broken JavaScript file somewhere.
I don’t see how schemas solve these problems. I’m not sure what problem you want to address with schemas?
from unified-language-server.
I don’t see how schemas solve these problems. I’m not sure what problem you want to address with schemas?
A schema would specifically address the issue mentioned in the title, testing for a broken configuration file.
The several examples you mention:
Some examples: a plugin throwing an error, a broken JavaScript file somewhere.
are all good things to account for, but aren't exactly a broken config?
This issue was particularly about missing unit tests. A comment in the code says something cannot occur, but it can, and should be tested.
Thanks for the clarification
from unified-language-server.
testing for a broken configuration file.
You mean broken remark or rehype config files? In that case, I do not yet understand the pros and cons of what you propose and how it differs from the current state (where broken config files result in errors), and how it would work in unified-language-server
(or unified-engine
? Or somewhere else?)
but aren't exactly a broken config?
While the title is short, it is expanded upon in the problem description. Certain errors are thrown when unified-engine
/ unified-language-server
are misconfigured, such as with missing reporters or when no input is given. These errors are real, and can be tested. The code currently says they never occur, and doesn’t test them.
from unified-language-server.
You mean broken remark or rehype config files?
I do
I do not yet understand the pros and cons of what you propose and how it differs from the current state
The main difference would be (for json and yaml configurations), we may be able to provide more detailed messages for invalid configuration, at the cost of including a json schema library
how it would work in unified-language-server (or unified-engine? Or somewhere else?)
It would be a validation step that would happen after configuration file is loaded, and before we attempt to interpret/run the configuration.
These errors are real, and can be tested. The code currently says they never occur, and doesn’t test them.
Gotcha, thanks for the clarification
from unified-language-server.
I think supporting JSON schema based validations might be interesting. I personally have some good experience using jsonschema
and yaml
to point users to the exact file and position in a file that’s invalid. I think this is also possible using jsonc-parser
.
ESLint also uses JSON schemas to validate plugin options. It could be done for unified plugins. It should be optional though, since many existing plugins don’t use it.
However, I don’t think this is what the issue is about. It’s about testing how unified-language-server
deals with broken configuration files.
from unified-language-server.
It might be useful but I’m not sure how much schema validation in the engine/unified-language-server adds compared to:
- existing types
- existing runtime errors when unexpected values are passed in plugins
Something prop-types/ESLint like feels verbose to me, also ’cause we already have the above 2.
But improving error messages/traces for broken/unexpected values in general are 👍 for me.
Not sure how to do it for JS config files? Perhaps ASTs even? More likely something with https://github.com/felixge/node-stack-trace / https://v8.dev/docs/stack-trace-api? Or not at all.
Generally also 👍 on adding tools to improve things to the engine (similar to editorconfig), but very cautious to add it to unified
itself.
As an aside, more for @remcohaszing, settings
in remarkrc/rehyperc in SchemaStore as you added them are likely useful, but not strictly correct (as anything can be passed in settings, and any plugin could read from them and expect whatever, and the default *parse and *stringify plugins could be swapped for something else). There’s also the uncommon case where remark -> rehype would be done in a config file. Not sure how to handle that.
from unified-language-server.
I think we’ve gone a bit off track with the discussion. The original issue is not about any form of schema validation, but about:
- Missing test coverage.
- Adding a test for handling broken coverage.
I added a test, but it doesn’t hit those lines. unified-engine
handles a broken config gracefully. So I don’t really think I need to push this, but I can if you want. You can see this easily by creating a file named .remarkrc.json
in a repo with the contents {
. Then open a markdown file with VSCode and vscode-remark
installed.
I don’t know what does cause unified-engine
to throw.
from unified-language-server.
incorrect options directly passed to engine
yields an error, such as: https://github.com/unifiedjs/unified-engine/blob/8fd1e22bd38be8a55bb2c3f70fa308cbc9d226a8/lib/index.js#L360.
from unified-language-server.
Hi! This was closed. Team: If this was fixed, please add phase/solved
. Otherwise, please add one of the no/*
labels.
from unified-language-server.
Related Issues (20)
- Automated Testing HOT 3
- Implement unified-engine HOT 2
- Add unified-language-server to vim-lsp-settings HOT 4
- Add documentation for more editors HOT 2
- `file.cwd` is not correctly set HOT 5
- Smarter file processing HOT 7
- Support vfile message `note` HOT 4
- The language server cwd is not a good fallback for `unified-engine` cwd HOT 7
- Incremental processing HOT 3
- Don’t perform actions when no configuration is found. HOT 21
- Fallback to global or bundled remark version HOT 4
- Make sure input URIs don’t change HOT 2
- Rewrite global console to output to the language server connection console HOT 6
- Allows defaults to `CWD` should support passing `additionalPaths` HOT 16
- Mark single expected suggestion as preferred HOT 2
- Clean stack traces HOT 5
- Support node ancesters via a configuration option HOT 3
- jsonrpc error thrown HOT 2
- Ability to specify configuration file as command-line argument HOT 3
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from unified-language-server.