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.

Sign up for GitHub

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

Jump to bottom

migrate from shellfish to rustyline with enhanced CLI features #6830

Merged
merged 21 commits into from
Jan 17, 2025
Merged

migrate from shellfish to rustyline with enhanced CLI features #6830

merged 21 commits into from
Jan 17, 2025

Conversation

JoshuaBatty
Copy link
Member

@JoshuaBatty JoshuaBatty commented Jan 13, 2025 •
edited
Loading

Description

This PR migrates the forc debug CLI from shellfish to rustyline, providing enhanced interactive debugging features. Key improvements include:

  • Command history with persistence
  • Intelligent command completion for debugger commands, register names, and transaction files
  • Command suggestions/corrections for typos
  • Contextual hints showing command descriptions

The migration also reorganizes the CLI code into separate modules for better maintainability while preserving all existing debugger functionality.

See video below for demo.

debug.cli.mp4

Checklist

  • I have linked to any relevant issues.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have updated the documentation where relevant (API docs, the reference, and the Sway book).
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added (or requested a maintainer to add) the necessary Breaking* or New Feature labels where relevant.
  • I have done my best to ensure that my PR adheres to the Fuel Labs Code Review Standards.
  • I have requested a review from the relevant team or maintainers.

@JoshuaBatty JoshuaBatty requested review from a team as code owners January 13, 2025 02:30
@JoshuaBatty JoshuaBatty self-assigned this Jan 13, 2025
@JoshuaBatty JoshuaBatty added the forc-debug Everything related to forc-debug label Jan 13, 2025
@JoshuaBatty JoshuaBatty marked this pull request as draft January 13, 2025 02:31
@JoshuaBatty JoshuaBatty marked this pull request as ready for review January 15, 2025 02:28
@JoshuaBatty JoshuaBatty requested a review from a team January 15, 2025 02:29
sdankel
sdankel previously approved these changes Jan 15, 2025
View reviewed changes
Copy link
Member

@sdankel sdankel left a comment

Choose a reason for hiding this comment

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

The new help message and completion are so slick! Nice work.

forc-plugins/forc-debug/src/cli/state.rs Outdated Show resolved Hide resolved
@JoshuaBatty JoshuaBatty requested a review from a team January 15, 2025 23:48
sdankel
sdankel previously approved these changes Jan 17, 2025
View reviewed changes
kayagokalp
kayagokalp previously approved these changes Jan 17, 2025
View reviewed changes
Copy link
Member

@kayagokalp kayagokalp left a comment

Choose a reason for hiding this comment

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

Looks good to me! Do we have any location that contains documentation about debugging? If so we might need to update those places as well with new screen shots etc

@JoshuaBatty
Copy link
Member Author

Looks good to me! Do we have any location that contains documentation about debugging? If so we might need to update those places as well with new screen shots etc

Good idea. Yeah we do, the documentation is pretty out of date though. My plan was to update that in a follow up PR with some work on decoding the log receipts.

@alfiedotwtf alfiedotwtf self-requested a review January 17, 2025 02:25
alfiedotwtf
alfiedotwtf requested changes Jan 17, 2025
View reviewed changes
Copy link
Contributor

@alfiedotwtf alfiedotwtf 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.

Oops. Muscle memory to approve. Just the comment change is needed, but other suggestion you can do it now or safely do in a future PR.

zees-dev
zees-dev previously approved these changes Jan 17, 2025
View reviewed changes
Copy link
Contributor

@zees-dev zees-dev left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@JoshuaBatty JoshuaBatty dismissed stale reviews from zees-dev, kayagokalp, and sdankel via 20ad867 January 17, 2025 04:07
alfiedotwtf
alfiedotwtf previously approved these changes Jan 17, 2025
View reviewed changes
@IGI-111 IGI-111 merged commit 9cd1b39 into master Jan 17, 2025
39 checks passed
@IGI-111 IGI-111 deleted the josh/debug_fixtures branch January 17, 2025 11:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alfiedotwtf alfiedotwtf alfiedotwtf approved these changes

@IGI-111 IGI-111 IGI-111 approved these changes

@zees-dev zees-dev zees-dev approved these changes

@sdankel sdankel Awaiting requested review from sdankel

@kayagokalp kayagokalp Awaiting requested review from kayagokalp

Assignees

@JoshuaBatty JoshuaBatty

Labels
forc-debug Everything related to forc-debug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@JoshuaBatty @alfiedotwtf @IGI-111 @kayagokalp @sdankel @zees-dev