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

callstack wip #108

Merged
merged 4 commits into from
Mar 22, 2024
Merged

callstack wip #108

merged 4 commits into from
Mar 22, 2024

Conversation

saimeunt
Copy link
Contributor

@saimeunt saimeunt commented Mar 18, 2024

WIP fixing #90

Screenshot 2024-03-19 at 00 29 15

@mazurroman this is a simple implementation where we show the callstack in the debug tab.
The FP button is clickable to jump to the frame pointer.
Have you something more complex in mind?

@mazurroman
Copy link
Contributor

@saimeunt nice progress ser!

One thing that comes to mind: can we format this as a table? So that fp is always in a single column. Will also be easier to understand that some values are missing in some rows; i.e. the first row in your screenshot has no call pc and no ret pc - currently this is not immediately clear.

@mazurroman mazurroman linked an issue Mar 19, 2024 that may be closed by this pull request
@saimeunt
Copy link
Contributor Author

@mazurroman A table is indeed more appropriate.

Screenshot 2024-03-19 at 18 26 02

@saimeunt saimeunt marked this pull request as ready for review March 19, 2024 17:28
@saimeunt saimeunt requested a review from a team as a code owner March 19, 2024 17:28
@mazurroman
Copy link
Contributor

looking really good @saimeunt one small suggestion: can you wrap the table into a border too? I also suggest to change all the borders to gray-300 to keep it consistent with borders of registers. See screenshot.

CleanShot 2024-03-20 at 10 48 25@2x

Copy link
Contributor

@mazurroman mazurroman left a comment

Choose a reason for hiding this comment

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

see my previous comment; lets just update the border colors and we are good to go!

@saimeunt
Copy link
Contributor Author

@mazurroman thanks for the design help, I'm definitely not an expert on this field! 😅

@mazurroman
Copy link
Contributor

looking great @saimeunt !

@mazurroman mazurroman self-requested a review March 22, 2024 13:15
@mazurroman mazurroman merged commit cd88fd8 into walnuthq:main Mar 22, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feat: Create call stack with cairo function names
2 participants