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

Initial work on adopting new parser #442

Merged
merged 16 commits into from
Oct 5, 2021
Merged

Initial work on adopting new parser #442

merged 16 commits into from
Oct 5, 2021

Conversation

gorkem
Copy link
Collaborator

@gorkem gorkem commented Apr 19, 2021

Replacing the parser with eemeli yaml parser AST to internal AST.

class CommonTagImpl {
public tag: string;
public readonly type: string;
default: never;
Copy link
Member

Choose a reason for hiding this comment

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

public is the default. If you left it off to make it private it will not work. It would be better to be consistent and use public on all public fields or none of them.

gorkem and others added 15 commits October 4, 2021 09:46
implements a new converter that converts eemeli
yaml parser AST to internal AST.
Fixes the conversion of warnings from
parser AST.
Implement a generic tag implementation to
support custom tags from settings.
Implements the include tag and validates for
empty values. Updates the test which gives a more
accurate range for error.
Updates the tests to new parser message
and range.
Updates tests that require YAML 1.1 behaviour to
include %YAML 1.1 directive.
* Fix "Billion Laughs" attack and validation tests

Signed-off-by: Yevhen Vydolob <[email protected]>

* Fix vslidation tests

Signed-off-by: Yevhen Vydolob <[email protected]>
Adjusts the duplicate key error tests to the
message from new parser. Also updates the
test for a single error instead of multiple
errors.
* Upgrade yaml parser to latest version

Signed-off-by: Yevhen Vydolob <[email protected]>

* and fix one more test

Signed-off-by: Yevhen Vydolob <[email protected]>
* Use new AST in completion

Signed-off-by: Yevhen Vydolob <[email protected]>

* Fix validation

Signed-off-by: Yevhen Vydolob <[email protected]>

* Add tests, fix review comments

Signed-off-by: Yevhen Vydolob <[email protected]>
@evidolob evidolob marked this pull request as ready for review October 4, 2021 08:06
@coveralls
Copy link

coveralls commented Oct 4, 2021

Coverage Status

Coverage decreased (-4.7%) to 73.615% when pulling 1cec73b on parser-switch into 67d85c4 on main.

@evidolob
Copy link
Collaborator

evidolob commented Oct 4, 2021

@gorkem @JPinkney I think we are ready to merge this.
Could you do final review/test?

@gorkem
Copy link
Collaborator Author

gorkem commented Oct 4, 2021

I can not really approve this PR since I am the author but please go ahead and merge.

@evidolob evidolob merged commit 4812ffe into main Oct 5, 2021
@gorkem gorkem deleted the parser-switch branch October 5, 2021 19:49
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.

4 participants