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

SSA Improvements #3892

Merged
merged 14 commits into from
Jul 6, 2023
Merged

SSA Improvements #3892

merged 14 commits into from
Jul 6, 2023

Conversation

joeizang
Copy link
Collaborator

@joeizang joeizang commented Jul 5, 2023

fixes #3882

  • Result tabs along with warning checkboxes should be shown on analysis running not on compilation
  • If all tools are unchecked, in result section, result is shown but not the tabs
  • Show warnings and hide warnings labels can be confusing for some users
  • Position on Solhint warnings should be improved
  • Text on badge tooltip can have count

@joeizang joeizang added the WIP label Jul 5, 2023
@joeizang joeizang self-assigned this Jul 5, 2023
@netlify
Copy link

netlify bot commented Jul 5, 2023

Deploy Preview for remixproject ready!

Name Link
🔨 Latest commit 23e6620
🔍 Latest deploy log https://app.netlify.com/sites/remixproject/deploys/64a6dc7ed1823300088cec75
😎 Deploy Preview https://deploy-preview-3892--remixproject.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@joeizang joeizang added ready-to-review PR ready to review and removed WIP labels Jul 5, 2023
@joeizang joeizang requested a review from Aniket-Engg July 5, 2023 10:58
Copy link
Collaborator

@Aniket-Engg Aniket-Engg left a comment

Choose a reason for hiding this comment

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

If I uncheck all boxes, and click on show errors only checkbox, badge shows message with count
Screenshot 2023-07-05 at 6 55 58 PM

@joeizang joeizang requested a review from Aniket-Engg July 5, 2023 22:44
result.errCol = position ? parseInt(position[3]) : -1
let position = msg.match(/^(.*?):([0-9]*?):([0-9]*?)?/);
result.errLine = position ? parseInt(position[2]) - 1 : -1;
result.errCol = position ? parseInt(position[3]) : -1;
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you find it fine, I think we can remove all semicolons

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah its the formatter I was experimenting with. missed removing those

@@ -198,12 +207,13 @@ slitherEnabled: boolean, setStartAnalysis: React.Dispatch<React.SetStateAction<b
props.analysisModule.call('terminal', 'log', { type: 'error', value: '[Slither Analysis]: Error occured! See remixd console for details.' })
showWarnings(warningMessage, 'warningModuleName')
}
} else showWarnings(warningMessage, 'warningModuleName')
} else //showWarnings(warningMessage, 'warningModuleName')
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove commented code

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

sure

}, [solhintEnabled, basicEnabled, slitherEnabled, showSlither])

useEffect(() => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is this for?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

just missed removing it

@joeizang joeizang merged commit 97c9408 into master Jul 6, 2023
6 checks passed
@joeizang joeizang deleted the ssa-improvements branch July 6, 2023 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-review PR ready to review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SSA improvements
2 participants