Skip to content

Conversation

ras0219
Copy link
Contributor

@ras0219 ras0219 commented Sep 7, 2020

This PR does a few things:

  1. Internally splits the representation (json.h) and parser (jsonreader.h) to reduce unintended dependencies across the codebase. The same refactor is also applied to Configuration/ConfigurationDeserializer.
  2. Introduces levenshtein-distance suggestions for misspelled fields.
  3. Minor tweak to the identifier error message.

Potential future work includes applying this levenshtein-distance calculation to search results.

As a drive by, this PR documents that comment fields ($xyz) are not supported in objects being used as sets with user-defined keys; they are only allowed in "struct-like" objects with a defined keyspace.

Example 1

{ "name": "Abseil", ... }
PS C:\src\vcpkg\testing> ..\vcpkg-old.exe install --feature-flags=manifests,registries --dry-run
Errors occurred while parsing C:\src\vcpkg\testing\vcpkg.json
    $.name: mismatched type: expected an identifier
See https://github.com/Microsoft/vcpkg/tree/master/docs/specifications/manifests.md for more information.
PS C:\src\vcpkg\testing> ..\vcpkg.exe install --feature-flags=manifests,registries --dry-run
Errors occurred while parsing C:\src\vcpkg\testing\vcpkg.json
    $.name (an identifier): must be lowercase alphanumeric+hyphens and not reserved
See https://github.com/Microsoft/vcpkg/tree/master/docs/specifications/manifests.md for more information.

Example 2

{ .., "depednecies": ["foo"], ... }
PS C:\src\vcpkg\testing> ..\vcpkg-old.exe install --feature-flags=manifests,registries --dry-run
Errors occurred while parsing C:\src\vcpkg\testing\vcpkg.json
    $ (a manifest): unexpected field 'depednecies'
See https://github.com/Microsoft/vcpkg/tree/master/docs/specifications/manifests.md for more information.
PS C:\src\vcpkg\testing> ..\vcpkg.exe install --feature-flags=manifests,registries --dry-run
Errors occurred while parsing C:\src\vcpkg\testing\vcpkg.json
    $ (a manifest): unexpected field 'depednecies', did you mean 'dependencies'?
See https://github.com/Microsoft/vcpkg/tree/master/docs/specifications/manifests.md for more information.

Example 3

{
  ...,
  "features": {
    "$abc" : true
  }
}
PS C:\src\vcpkg\testing> ..\vcpkg-old.exe install --feature-flags=manifests,registries --dry-run
Errors occurred while parsing C:\src\vcpkg\testing\vcpkg.json
    $.features (a features field): unexpected field '$abc'
See https://github.com/Microsoft/vcpkg/tree/master/docs/specifications/manifests.md for more information.
PS C:\src\vcpkg\testing> ..\vcpkg.exe install --feature-flags=manifests,registries --dry-run
Errors occurred while parsing C:\src\vcpkg\testing\vcpkg.json
    $.features (a set of features): unexpected field '$abc': must be lowercase alphanumeric+hyphens and not reserved
See https://github.com/Microsoft/vcpkg/tree/master/docs/specifications/manifests.md for more information.

@PhoebeHui PhoebeHui added the category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed label Sep 8, 2020
@strega-nil strega-nil self-requested a review September 8, 2020 17:02
@strega-nil
Copy link
Contributor

Putting myself as a required reviewer.

@ras0219 ras0219 requested a review from strega-nil September 10, 2020 10:08
Copy link
Contributor

@strega-nil strega-nil left a comment

Choose a reason for hiding this comment

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

Just a few things left :)

@ras0219 ras0219 requested a review from strega-nil October 9, 2020 08:59
@ras0219-msft ras0219-msft merged commit d1ba685 into microsoft:master Oct 12, 2020
strega-nil pushed a commit to strega-nil/vcpkg that referenced this pull request May 5, 2021
* [vcpkg] Split vcpkg/base/json.h into vcpkg/base/jsonreader.h

* [vcpkg] Extract definitions of Configuration-Deserializer (& friends)

These types are only used by VcpkgPaths during the initial parse.

* [vcpkg] Introduce levenshtein-distance suggestions for json errors

* [vcpkg] Fix regression in supports handling

* [vcpkg] Fix signed/unsigned mismatch

* [vcpkg] Address CR comments

* [vcpkg] Address CR comments

* Fix compiler error from merge conflict.

* [vcpkg] Change parameters of Reader::check_for_unexpected_fields to better match declaration

* [vcpkg] Improve errors from features set

* [vcpkg] Fix includes

* [vcpkg] Reuse code

* [vcpkg] Check the "name" field always to maximize error information

* [docs] Improve english phrasing in manifests.md

* [vcpkg] Correct docs link for manifests

Co-authored-by: Robert Schumacher <[email protected]>
Co-authored-by: Billy Robert O'Neal III <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants