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.

Sign up for GitHub

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

Jump to bottom

Improve error messages #49

Merged
merged 9 commits into from
Sep 1, 2021
Merged

Improve error messages #49

merged 9 commits into from
Sep 1, 2021

Conversation

kpodsiad
Copy link
Contributor

@kpodsiad kpodsiad commented Aug 2, 2021 •
edited
Loading

TODO:

  • add Input to the Position like it is in e.g scalameta trees Position
  • use Position to create error messages like the following. Maybe not all of them would be done in this PR but I want to implement at least one of them.
Error: <line>:<column> expected value in mapping, but got nothing instead
key: <nothing>
     ^^^^^^^^^

Error: <line>:<column> expected int, got string instead
key: "string_value"
     ^^^^^^^^^^^^^^
  • My changes broke all the tests and I have to do sth to fix this.

@kpodsiad kpodsiad added the feature New feature or request label Aug 2, 2021
@kpodsiad kpodsiad added this to the M2 milestone Aug 2, 2021
Base automatically changed from reader_position to main August 2, 2021 18:23
@kpodsiad kpodsiad added this to In progress in scala-yaml Aug 28, 2021
Kamil Podsiadlo added 3 commits August 31, 2021 11:45
Add position to token, event and nodes
dc68d0d
Fix compilatin errors
6443259
Enhance error messages
f8eb5ee 
Add position, indicate invalid input and show expected type
Fix test
544b386 
Add BaseParserSuite class which provides method for asserting events
@kpodsiad kpodsiad changed the title WIP: Feature/errors improvement Feature/errors improvement Aug 31, 2021
@kpodsiad kpodsiad marked this pull request as ready for review August 31, 2021 11:42
@kpodsiad kpodsiad changed the title Feature/errors improvement Improve error messages Aug 31, 2021
lwronski
lwronski reviewed Aug 31, 2021
View reviewed changes
Copy link
Contributor

@lwronski lwronski left a comment

Choose a reason for hiding this comment

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

LGTM, waiting for minor corrections

yaml/shared/src/main/scala/org/virtuslab/yaml/internal/load/compose/Composer.scala Outdated Show resolved Hide resolved
yaml/shared/src/main/scala/org/virtuslab/yaml/internal/load/compose/Composer.scala Outdated Show resolved Hide resolved
lwronski
lwronski approved these changes Sep 1, 2021
View reviewed changes
Copy link
Contributor

@lwronski lwronski left a comment

Choose a reason for hiding this comment

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

LGTM, squash commits before merge

@kpodsiad kpodsiad merged commit 99ef506 into main Sep 1, 2021
scala-yaml automation moved this from In progress to Done Sep 1, 2021
@kpodsiad kpodsiad deleted the feature/errors_improvement branch September 1, 2021 08:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@romanowski romanowski romanowski left review comments

@lwronski lwronski lwronski approved these changes

Assignees
No one assigned
Labels
feature New feature or request
Projects
No open projects
scala-yaml
Done
Milestone
M2
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@kpodsiad @romanowski @lwronski