Skip to content

Improve some things around how errors are reported when parsing a file - #177

Merged
sbinet merged 2 commits into
go-python:masterfrom
Tatskaari:fix-error-logging
May 5, 2022
Merged

Improve some things around how errors are reported when parsing a file#177
sbinet merged 2 commits into
go-python:masterfrom
Tatskaari:fix-error-logging

Conversation

@Tatskaari

@TatskaariTatskaari commented Apr 10, 2022

Copy link
Copy Markdown

Given the malformed input "/Users/jpoole/gpython/test.py":

def foo(name : str):
return name
foo(
foo(name = "test"))
foo(

Before:

Exception &py.Exception{Base:(*py.Type)(0xc00012dc20), Args:py.Tuple{"invalid syntax"}, Traceback:py.Object(nil), Context:py.Object(nil), Cause:py.Object(nil), SuppressContext:false, Dict:py.StringDict{"filename":"<string>", "line":"", "lineno":10, "offset":0}}
-- No traceback available --
2022/04/10 18:31:19 File "<string>", line 10, offset 0
SyntaxError: 'invalid syntax'

After

Exception File "/Users/jpoole/gpython/test.py", line 10, offset 0
SyntaxError: 'unexpected EOF while parsing'
-- No traceback available --

@sbinetsbinet left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

apologies for dropping the ball.

LGTM, modulo some tests that need to be fixed.

thanks for the PR.

@Tatskaari

Copy link
Copy Markdown
Author

apologies for dropping the ball.

LGTM, modulo some tests that need to be fixed.

thanks for the PR.

Nice! Fixed the tests up. They pass locally now.

@sbinetsbinet left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM.

thanks for sticking with us.

could you send another PR against go-python/license adding yourself to the AUTHORS and/or CONTRIBUTORS file(s) ?

then I'll merge this one in.

thanks again.

@sbinet

Copy link
Copy Markdown
Member

there's one remaining error, though:

Traceback (most recent call last):
File "<stdin>", line 1, in <module>
NameError: "name 'aksfjakf' is not defined"
--- FAIL: TestREPL (0.00s)
repl_test.go:32: compileError: Output wrong, want "Compile error: \n File \"<string>\", line 1, offset 2\n if\n\n\nSyntaxError: 'invalid syntax'" got "Compile error: \n File \"<stdin>\", line 1, offset 2\n if\n\n\nSyntaxError: 'invalid syntax'"
FAIL
FAIL	github.com/go-python/gpython/repl	0.003s

@sbinet

Copy link
Copy Markdown
Member

needs go-python/license#17

Jon Pooleand others added 2 commits May 5, 2022 14:25
@sbinet
sbinetforce-pushed the fix-error-logging branch from 4b82bb3 to 02c67b2CompareMay 5, 2022 12:26
@sbinet

Copy link
Copy Markdown
Member

I took the liberty of fixing the last remaining error and pushing my changes, so I could merge this PR in :)

thanks a lot.

@codecov

codecovBot commented May 5, 2022

Copy link
Copy Markdown

Codecov Report

Merging #177 (02c67b2) into master (12886a2) will not change coverage.
The diff coverage is 40.00%.

@@ Coverage Diff @@## master #177 +/- ##
=======================================
Coverage 73.22% 73.22% =======================================
Files 72 72 Lines 11765 11765 =======================================
Hits 8615 8615 Misses 2568 2568 Partials 582 582 
Impacted FilesCoverage Δ
main.go15.62% <0.00%> (ø)
py/traceback.go22.22% <0.00%> (ø)
compile/compile.go90.56% <100.00%> (ø)
parser/lexer.go89.03% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 12886a2...02c67b2. Read the comment docs.

@sbinetsbinet left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@sbinet
sbinet merged commit 23774dd into go-python:masterMay 5, 2022
@Tatskaari

Copy link
Copy Markdown
Author

I took the liberty of fixing the last remaining error and pushing my changes, so I could merge this PR in :)

thanks a lot.

Oh so sorry! I got distracted and forgot to get back to this one. Thanks for that :)

@sbinet

Copy link
Copy Markdown
Member

no worries :)
(it'd be great to have python-3.x grammar compatibility (where x>4) :P)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@Tatskaari@sbinet