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

fix(yaml): update parsers #5027

Merged
merged 4 commits into from Sep 1, 2018

Conversation

Projects
None yet
3 participants
@ikatyang
Member

ikatyang commented Aug 30, 2018

Fixes #4991

  • upgrade to yaml@1.0.0-rc.8 and yaml-unist-parser@1.0.0-rc.4
  • refactor some logic since the AST has slightly changed (ikatyang/yaml-unist-parser#82)
  • unmatched aliases are now errors since it may introduce invalid AST from yaml
  • rewrite the document separator (.../---) logic, this fixes some cases where it can use --- but we printed ...
  • removed some unnecessary duplicate trailing newline
  • trailing comments on document (... #comment) and documentHead (--- #comment) are preserved (i.e. they won't be moved somewhere)

  • I’ve added tests to confirm my change works.
  • (If changing the API or CLI) I’ve documented the changes I’ve made (in the docs/ directory)
  • I’ve read the contributing guidelines.

ikatyang added some commits Aug 30, 2018

@marcofranssen

This comment was marked as off-topic.

marcofranssen commented Aug 30, 2018

The comment approach mentioned above is a workarround. Many kubernetes deployment files generated by tools or available in OSS out there don't have those comments. Meaning if I need to do small change I have to add this unnecessary comment all over the place. Can't we just allow these type of document headers?

E.g.

---
some_yaml:
   some_stuff: abcd
   more_stuff: ecde

---
some_other_yaml:
   awesomeness: 123
   cool:
     - name: daddy
     - name: mommy

This is some pseudo yamls to explain how many of those kubernetes files are usually available.

@ikatyang

This comment was marked as off-topic.

Member

ikatyang commented Aug 30, 2018

Could you move your comment to #5013? as this PR is not aimed to fix that issue, I just found it might be a solution. We should discuss it in #5013.

EDIT: moved to #5013 (comment)

@ikatyang ikatyang merged commit db2bc36 into prettier:master Sep 1, 2018

9 of 10 checks passed

codecov/project 96.53% (-0.03%) compared to 1657420
Details
ci/circleci: build_prod Your tests passed on CircleCI!
Details
ci/circleci: checkout_code Your tests passed on CircleCI!
Details
ci/circleci: test_prod_node4 Your tests passed on CircleCI!
Details
ci/circleci: test_prod_node9 Your tests passed on CircleCI!
Details
ci/circleci: test_prod_standalone Your tests passed on CircleCI!
Details
codecov/patch 96.42% of diff hit (target 80%)
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
deploy/netlify Deploy preview ready!
Details

@ikatyang ikatyang deleted the ikatyang:fix/yaml/update-parsers branch Sep 1, 2018

@ikatyang ikatyang added this to the 1.15 milestone Oct 25, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment