[PATCH] D140462: [clangd] Add schema for `.clangd` config

2023-01-10 Thread Edwin Kofler via Phabricator via cfe-commits
hyperupcall abandoned this revision. hyperupcall added a comment. I think I abandon this revision for the reasons stated above - there is an improved path forward at https://reviews.llvm.org/D140745 with autogenerated files. Reposito

[PATCH] D140462: [clangd] Add schema for `.clangd` config

2023-01-10 Thread Edwin Kofler via Phabricator via cfe-commits
hyperupcall updated this revision to Diff 487706. hyperupcall added a comment. Improve schema correctness and disable `additionalProperties` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140462/new/ https://reviews.llvm.org/D140462 Files: clang-

[PATCH] D140462: [clangd] Add schema for `.clangd` config

2023-01-09 Thread Edwin Kofler via Phabricator via cfe-commits
hyperupcall marked 2 inline comments as done. hyperupcall added a comment. In D140462#4018983 , @sammccall wrote: > The difficulty in testing is that: > > - JSON-schema is totally unknown in llvm-project, so there are no tools but > also no expectation t

[PATCH] D140462: [clangd] Add schema for `.clangd` config

2022-12-28 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments. Comment at: clang-tools-extra/clangd/schema/config.json:20 + "properties": { +"If": { + "description": "Conditions in the If block restrict when a fragment applies.", nridge wrote: > sammccall wrote: > > disabling `addi

[PATCH] D140462: [clangd] Add schema for `.clangd` config

2022-12-28 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. In D140462#4018983 , @sammccall wrote: > I tried 3 consumers (`ajv-cli`, `yaml-language-server`, and `yajsv`) and > **hit different blocking bugs in all of them** when using obvious, > spec-compliant approaches to writing schemas

[PATCH] D140462: [clangd] Add schema for `.clangd` config

2022-12-28 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. The difficulty in testing is that: - JSON-schema is totally unknown in llvm-project, so there are no tools but also no expectation that contributors understand it - llvm-project doesn't like dependencies, particularly those that aren't in C++ or python - having done s

[PATCH] D140462: [clangd] Add schema for `.clangd` config

2022-12-26 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. In D140462#4017240 , @nridge wrote: > Is there a command-line tool that can perform JSON schema validation (well, more specifically that can validate YAML files against a JSON schema) Repository: rG LLVM Github Monorepo CHANG

[PATCH] D140462: [clangd] Add schema for `.clangd` config

2022-12-26 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. In D140462#4017237 , @hyperupcall wrote: > If you would like me to add tests to verify the schema (for now or later?), > is there a utility within LLVM to help do so? I saw TableGen was mentioned, > but it sounds like the schema

[PATCH] D140462: [clangd] Add schema for `.clangd` config

2022-12-26 Thread Edwin Kofler via Phabricator via cfe-commits
hyperupcall added a comment. Thank you for the suggestions - I applied all of the fixes! As someone who helps maintain a lot of the schemas on schemastore, it's nice when schemas are in-tree with their respective project, but I totally understand th

[PATCH] D140462: [clangd] Add schema for `.clangd` config

2022-12-26 Thread Edwin Kofler via Phabricator via cfe-commits
hyperupcall updated this revision to Diff 485352. hyperupcall added a comment. Apply `clang-format` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140462/new/ https://reviews.llvm.org/D140462 Files: clang-tools-extra/clangd/schema/config.json In

[PATCH] D140462: [clangd] Add schema for `.clangd` config

2022-12-26 Thread Edwin Kofler via Phabricator via cfe-commits
hyperupcall updated this revision to Diff 485351. hyperupcall added a comment. Improve schema correctness Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140462/new/ https://reviews.llvm.org/D140462 Files: clang-tools-extra/clangd/schema/config.js

[PATCH] D140462: [clangd] Add schema for `.clangd` config

2022-12-23 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. In D140462#4014944 , @sammccall wrote: > given lack of any automation/tests That's a fair point, it would definitely help validate the correctness of the schema if we had tests in the form of example config files that pass or fai

[PATCH] D140462: [clangd] Add schema for `.clangd` config

2022-12-23 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. I think there's a clear benefit, but my feeling is it's outweighed by the costs, which are fairly high given lack of any automation/tests. If clangd contributors should maintain this, how should contributors/reviewers know whether changes are correct? If they needn't m

[PATCH] D140462: [clangd] Add schema for `.clangd` config

2022-12-22 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment. In D140462#4012996 , @sammccall wrote: > This has been discussed before, unfortunately while I *thought* it was on a > bug, it was actually in a PR: https://github.com/clangd/vscode-clangd/pull/140 > > I don't think it's a good id

[PATCH] D140462: [clangd] Add schema for `.clangd` config

2022-12-22 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. In D140462#4012451 , @nridge wrote: > Thanks, this is pretty neat! > > cc @sammccall as a heads up, just to make sure you're cool with having this > in-tree This has been discussed before, unfortunately while I *thought* it wa

[PATCH] D140462: [clangd] Add schema for `.clangd` config

2022-12-22 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a subscriber: sammccall. nridge added a comment. Thanks, this is pretty neat! cc @sammccall as a heads up, just to make sure you're cool with having this in-tree Comment at: clang-tools-extra/clangd/schema/config.json:48 + "description": "Directory to se

[PATCH] D140462: [clangd] Add schema for `.clangd` config

2022-12-20 Thread Edwin Kofler via Phabricator via cfe-commits
hyperupcall created this revision. Herald added subscribers: kadircet, arphaman. Herald added a project: All. hyperupcall requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. Fixes https://github.com/clangd/cla