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

Errors should be "click on-able" #170

Open
PatrickAlphaC opened this issue Dec 2, 2024 · 4 comments
Open

Errors should be "click on-able" #170

PatrickAlphaC opened this issue Dec 2, 2024 · 4 comments

Comments

@PatrickAlphaC
Copy link
Member

Putting them in a temp folder makes it difficult to view them in my text editor.

2024-12-02 08 47 39

@s3bc40
Copy link
Contributor

s3bc40 commented Jan 29, 2025

I have a proposal for a solution in there: #196

I struggled a little to find a way to avoid any duplicated code, to be able to act at one point only. But the only way that gave me some results is from my PR: by passing the original path from a fixture and apply a utility helper to remap the path from the tracebacks.

I did it for the cli deployments script, but if it is an accepted solution, I can do the others. (I'll try to do them later)

Also, not really satisfied by the fact that it can duplicate the console output (one with the original and the temp), since traceback is immutable, apparently.


EDIT:

I reworked what I did so... Let's see if it fits the requirements!

@PatrickAlphaC
Copy link
Member Author

Thanks for working on this!

It looks like there is actually a lot of good stuff we can use in this PR! Some nice reworking of these tests - they were pretty messy and this is a nice clean up.

I'm struggling to see how this fixing the issue though? I'm a bit upset I didn't give enough direction on this :(

What the issue is trying to solve, is that when you run something like:

mox compile

And you get an error, the error is "clickable" meaning you can CMD + Click the line in the error which will bring you to the line in the contract that has the issue.

What do you think? I feel like we should merge in your refactor where you clean up the architecture, but keep the issue open with more details?

@s3bc40
Copy link
Contributor

s3bc40 commented Jan 31, 2025

Yeah my bad on this, I jumped into my assumptions that it was related to tempfile.TemporaryDirectory() inside the test suite. I should have asked first, even though I managed to understand what could be the issue, and maybe we should just catch the specific error and apply the restore_original_path_in_error in mox compile

We can add what I have done as a refacto, but I think I'll remove the restore_original_path_in_error from the tests.utils since it might be overkilled with pytest wrapping all the error and log output.

I'll try to add restore_original_path_in_error logic in the right solution for this issue if I pin down the moment I can catch the error.


I think I'll close this PR #196 and cleanup the commit history from the wip and reworks. It will be nicer to arrange this in one clean commit. Also renaming the branch will be better I think...

Then I will come up with a solution for this issue specifically! 👍

@s3bc40
Copy link
Contributor

s3bc40 commented Jan 31, 2025

On my Linux, I still have the clear path of the error after mox compile. When I Ctrl + Click on this error from vscode, for example:

vyper.exceptions.StructureException: Expressions without assignment are disallowed

  (hint: did you mean to assign the result to a variable?)

  contract "/home/<PATH>/my_project/src/Counter.vy:9", function "set_number", line 9:4 
       8     self.number = new_number
  ---> 9     hello
  -----------^
      10

It opens the right file, but maybe there is a specific way to trigger this... 🤔

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

No branches or pull requests

2 participants