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

Ensure parser always returns erl_anno:location() in errors #325

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

gomoripeti
Copy link
Contributor

This way the return type matches error_info()

This is an alternative fix for #324

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Nov 26, 2021
@michalmuskala
Copy link
Member

michalmuskala commented Dec 10, 2021

Hey, sorry for taking so long to reply on this

I think it would be better to do the reverse - to make sure, if possible, we always return the richer types on errors. If anything the errors are more informative with potential start & end location of the error, and not just start one.

@gomoripeti
Copy link
Contributor Author

gomoripeti commented Dec 10, 2021

thanks for the feedback

some pros for the simple location

Which one would you prefer?

  • just allow both location and anno in the type of erlfmt:error_info() (as in Improve error_info() type #324)?
  • modify erlfmt_scan to also return an anno (end_location is a mandatory field of erlfmt_scan:anno so then end_location = start_location?). also modify the formatting code (which I am absolutely note familiar with)
  • modify erlfmt module only to unify the error_infos coming from different components

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants