Skip to content

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

Merged
sbinet merged 2 commits intogo-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 intogo-python:masterfrom
Tatskaari:fix-error-logging

Conversation

@Tatskaari
Copy link

@Tatskaari Tatskaari commented Apr 10, 2022 •
edited
Loading

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 --

sbinet
sbinet reviewed Apr 20, 2022
View reviewed changes
Copy link
Member

@sbinet sbinet left a comment •
edited
Loading

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
Author

Tatskaari commented Apr 22, 2022

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.

sbinet
sbinet approved these changes Apr 22, 2022
View reviewed changes
Copy link
Member

@sbinet sbinet left a comment

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
Member

sbinet commented Apr 22, 2022

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
Member

sbinet commented Apr 22, 2022

needs go-python/license#17

@sbinet sbinet mentioned this pull request Apr 22, 2022
Merged
@sbinet sbinet force-pushed the fix-error-logging branch from 4b82bb3 to 02c67b2 Compare May 5, 2022 12:26
@sbinet
Copy link
Member

sbinet commented May 5, 2022

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
Copy link

codecov bot commented May 5, 2022

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 Files Coverage Δ
main.go 15.62% <0.00%> (ø)
py/traceback.go 22.22% <0.00%> (ø)
compile/compile.go 90.56% <100.00%> (ø)
parser/lexer.go 89.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.

sbinet
sbinet approved these changes May 5, 2022
View reviewed changes
Copy link
Member

@sbinet sbinet left a comment

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:master May 5, 2022
@Tatskaari
Copy link
Author

Tatskaari commented May 5, 2022

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
Member

sbinet commented May 5, 2022

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

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

Reviewers

@sbinet sbinet sbinet approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Tatskaari @sbinet