Skip to content

fix(cli): add pinocchio to Framework::FromStr and replace todo!() with unimplemented!()#689

Open
gamandeepsingh wants to merge 2 commits into
solana-foundation:mainfrom
gamandeepsingh:main
Open

fix(cli): add pinocchio to Framework::FromStr and replace todo!() with unimplemented!()#689
gamandeepsingh wants to merge 2 commits into
solana-foundation:mainfrom
gamandeepsingh:main

Conversation

@gamandeepsingh
Copy link
Copy Markdown

@gamandeepsingh gamandeepsingh commented Jun 5, 2026

Problem

Framework::FromStr in crates/cli/src/types/mod.rs had two issues:

  • "pinocchio" was missing from the match, so --framework pinocchio always
    returned Unknown framework: pinocchio even though Framework::Pinocchio
    is fully implemented everywhere else.
  • The Typhoon arms in both template methods used todo!(), which gives a
    misleading "not yet implemented" panic with no context.

Changes

  • Add "pinocchio" => Ok(Framework::Pinocchio) to FromStr
  • Replace todo!() with unimplemented!("Typhoon framework templates are not yet implemented")
    in both get_interpolated_program_deployment_template and
    get_in_memory_interpolated_program_deployment_template

Why unimplemented!() over todo!()

todo!() implies the code is actively being worked on.
unimplemented!() correctly signals that this feature doesn't exist yet,
giving a clear message if someone calls it instead of a bare panic.

Test plan

  • cargo test -p surfpool-cli — 18/18 passing
  • cargo +nightly fmt --all -- --check — clean
  • cargo check -p surfpool-cli — clean
  • cargo clippy --all-targets — 0 new warnings (pre-existing warnings unrelated to this change)

Closes #688

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Jun 5, 2026

Greptile Summary

This PR fixes two bugs in crates/cli/src/types/mod.rs: a missing "pinocchio" arm in Framework::FromStr that caused the CLI flag --framework pinocchio to always return an error, and two todo!() calls replaced with descriptive unimplemented!() panics in the Typhoon template methods.

  • Adds "pinocchio" => Ok(Framework::Pinocchio) to FromStr, aligning it with the existing Display impl and the enum definition.
  • Replaces bare todo!() with unimplemented!("Typhoon framework templates are not yet implemented") in both get_interpolated_program_deployment_template and get_in_memory_interpolated_program_deployment_template.

Confidence Score: 5/5

Safe to merge — the changes are minimal, targeted, and correct.

Both fixes are straightforward: the missing "pinocchio" arm in FromStr now matches the existing Display impl and enum variant, and the unimplemented!() replacements carry clearer diagnostic messages. No logic is altered for any working framework path, and the Typhoon path was already a panic regardless.

No files require special attention.

Important Files Changed

Filename Overview
crates/cli/src/types/mod.rs Bug fix: adds missing "pinocchio" arm to FromStr and improves Typhoon panic messages from todo!() to descriptive unimplemented!()

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["CLI: --framework <value>"] --> B["Framework::from_str(s)"]
    B --> C{"match s.to_lowercase()"}
    C -->|"anchor"| D[Framework::Anchor]
    C -->|"native"| E[Framework::Native]
    C -->|"steel"| F[Framework::Steel]
    C -->|"pinocchio"| G["Framework::Pinocchio (added)"]
    C -->|"typhoon"| H[Framework::Typhoon]
    C -->|"_"| I["Err: Unknown framework"]
    D & E & F & G --> J["get_interpolated_program_deployment_template()"]
    H --> K["unimplemented!() (was todo!())"]
    J --> L["Returns template String"]
Loading

Reviews (1): Last reviewed commit: "Refactor: Ran fmt command" | Re-trigger Greptile

Copy link
Copy Markdown
Collaborator

@MicaiahReid MicaiahReid left a comment

Choose a reason for hiding this comment

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

Good find, thanks @gamandeepsingh!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(cli): Framework::FromStr is incomplete — pinocchio missing, typhoon panics

2 participants