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

fix: informative Stubs when document is invalid #1068

Conversation

catosaurusrex2003
Copy link
Contributor

@catosaurusrex2003 catosaurusrex2003 commented Oct 6, 2024

Description
Diagnostics with severity 0 are displayed as errors.

Before
(Invalid document 👇 with syntax error)
image

After
(Invalid document 👇 with syntax error)
image

(document with spec specific errors 👇)
image

Related Issue
#1048
#874

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.

@catosaurusrex2003 catosaurusrex2003 changed the title fix: Informative Stubs when document is invalid fix: informative Stubs when document is invalid Oct 6, 2024
@catosaurusrex2003
Copy link
Contributor Author

The line in diagnostics being returned by the parser is not accurate when there are multi-line strings or when the lines move back indentations.

@derberg should we include the line number in the errors ?

example: below error should be on line 12, but it is showing on line 7 cuz it ignored the multi-lined string of /info/description
image

@catosaurusrex2003
Copy link
Contributor Author

catosaurusrex2003 commented Oct 7, 2024

I upgraded the package @asyncapi/parser from 3.1.0 -> 3.3.0
and now the error lines are reliable and working correctly

@AceTheCreator AceTheCreator self-requested a review October 7, 2024 14:39
Copy link

sonarcloud bot commented Oct 7, 2024

Copy link
Member

@AceTheCreator AceTheCreator left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@AceTheCreator
Copy link
Member

/rtm

@asyncapi-bot asyncapi-bot merged commit 31c2a86 into asyncapi:master Oct 7, 2024
10 checks passed
AceTheCreator added a commit that referenced this pull request Oct 7, 2024
@AceTheCreator
Copy link
Member

@catosaurusrex2003, i noticed something after merging this PR that the error line seem not to be acccurate for some of the errors just as seen below. It should be line 3 not line 4 🤔
AsyncAPI-React-Playground-App-10-07-2024_06_34_PM

@AceTheCreator
Copy link
Member

Wanna fix this in a follow-up PR?

@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 2.2.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

@catosaurusrex2003
Copy link
Contributor Author

@AceTheCreator I would love to work on this. But I cant seem to reproduce this issue again.

i tried the below on https://asyncapi.github.io/asyncapi-react/
image
and it is giving correct line number error

could it be you are running it locally without reinstalling the nodemodules again, cuz our @asyncapi/parser dependency version changed in the pr.

@AceTheCreator
Copy link
Member

Yea, that was the reason 😄. Everything looks good 🚀

Thanks for the fix 👍🏾

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

Successfully merging this pull request may close these issues.

4 participants