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 out of range panic #467

Closed
wants to merge 1 commit into from
Closed

Conversation

AdamKorcz
Copy link

Before submitting your PR, please confirm the following.

This fixes an out of range panic.

  • Describe the purpose for which you created this PR.
  • [] Create test code that corresponds to the modification

Signed-off-by: Adam Korczynski <[email protected]>
@codecov-commenter
Copy link

codecov-commenter commented Jul 22, 2024

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.77%. Comparing base (237df0e) to head (ac805ee).
Report is 1 commits behind head on master.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #467   +/-   ##
=======================================
  Coverage   78.77%   78.77%           
=======================================
  Files          13       13           
  Lines        4004     4004           
=======================================
  Hits         3154     3154           
  Misses        591      591           
  Partials      259      259           

@@ -333,7 +333,7 @@ func Integer(tk *token.Token) *IntegerNode {
}
negativePrefix = "-"
} else {
if value[1] == 'o' {
if len(value) >= 2 && value[1] == 'o' {

Choose a reason for hiding this comment

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

Could you add a unit test that reproduce the panic issue?

@goccy goccy added the reviewed label Oct 28, 2024
Copy link
Owner

@goccy goccy left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution, Please add the test case 🙏

@goccy
Copy link
Owner

goccy commented Nov 5, 2024

This problem has already been fixed with latest release. Thank you.

@goccy goccy closed this Nov 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants