Skip to content

Conversation

@theduke
Copy link
Contributor

@theduke theduke commented Oct 24, 2025

Cleans up the CLI run command code a bit by splitting up code into submodules,
improving code flow for the progress bar, and replacing the is-terminal crate
with standard library functionality.

Best reviewed commit by commit.

  • chore(cli): commands/run code quality cleanup
  • chore(cli): MonitoringRuntime progress bar cleanup
  • chore(deps): Replace is-terminal crate with stdlib

Dropped:
chore(cli): Only show compilation progress bar after a delay

Because it doesn't work correctly.

@theduke theduke requested a review from syrusakbary as a code owner October 24, 2025 04:38
@theduke theduke requested review from Arshia001 and marxin October 24, 2025 04:38
@theduke theduke force-pushed the cli-progressbar-fix branch 2 times, most recently from 64079c3 to 9535733 Compare October 24, 2025 04:55
@marxin
Copy link
Contributor

marxin commented Oct 24, 2025

The progress bar is really neat, I like it! For the future, we might want to integrate detailed bar update for Downloading and Compiling phases.

@theduke theduke force-pushed the cli-progressbar-fix branch from 9535733 to 2619307 Compare October 30, 2025 16:58
Copilot AI review requested due to automatic review settings October 30, 2025 16:58
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the CLI code to replace the external is-terminal crate with the standard library's IsTerminal trait, and reorganizes the wasmer run command implementation by extracting code into separate modules.

  • Replaces is-terminal crate dependency with std::io::IsTerminal from the standard library
  • Extracts PackageSource, ExecutableTarget, and MonitoringRuntime from run/mod.rs into separate module files (package_source.rs, target.rs, runtime.rs)
  • Adds a new helper method is_quiet_or_no_tty() to the Output struct for better encapsulation of quiet mode logic
  • Renames internal PackageSource enum to CliPackageSource to avoid naming conflict with wasmer_config::package::PackageSource

Reviewed Changes

Copilot reviewed 25 out of 26 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
lib/cli/Cargo.toml Removed is-terminal dependency from CLI package
lib/cli-compiler/Cargo.toml Removed is-terminal dependency from CLI compiler package
Cargo.lock Removed is-terminal from dependency tree
lib/cli/src/logging.rs Replaced is-terminal::IsTerminal with std::io::IsTerminal, added is_quiet_or_no_tty() helper method
lib/cli/src/commands/run/mod.rs Refactored to use new modules, updated references from PackageSource to CliPackageSource, updated to use is_quiet_or_no_tty()
lib/cli/src/commands/run/package_source.rs New module containing CliPackageSource enum (extracted from mod.rs)
lib/cli/src/commands/run/target.rs New module containing ExecutableTarget and TargetOnDisk enums (extracted from mod.rs)
lib/cli/src/commands/run/runtime.rs New module containing MonitoringRuntime and related structs (extracted from mod.rs)
lib/cli/src/commands/run/wasi.rs Updated import to use CliPackageSource
lib/cli/src/commands/run/capabilities/*.rs Updated references from PackageSource to CliPackageSource
lib/cli/src/commands/package/*.rs Replaced is-terminal::IsTerminal with std::io::IsTerminal
lib/cli/src/commands/app/**/*.rs Replaced is-terminal::IsTerminal with std::io::IsTerminal
lib/cli/src/commands/auth/logout.rs Replaced is-terminal::IsTerminal with std::io::IsTerminal
lib/cli/src/commands/mod.rs Replaced is-terminal::IsTerminal usage with std library version
lib/cli-compiler/src/utils.rs Replaced is-terminal::IsTerminal with std::io::IsTerminal

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.


/// Special wasix runtime implementation for the CLI.
///
/// Wraps an undelrying runtime and adds progress monitoring for package
Copy link

Copilot AI Oct 30, 2025

Choose a reason for hiding this comment

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

Corrected spelling of 'undelrying' to 'underlying'.

Suggested change
/// Wraps an undelrying runtime and adds progress monitoring for package
/// Wraps an underlying runtime and adds progress monitoring for package

Copilot uses AI. Check for mistakes.
The mod.rs is long and contains a lot of things.

This commit:
* splits funcitonality out into submodules (targetrs, runtime.rs,
  package_source.rs)
* Renames the shadowing PackageSource to CliPackageSource to avoid the
  renamed import and name confusion
* Fixes some other small code quality issues (eg Self vs <name> in impls)
* Use a single progress bar
* Show bar again if it was hidden
* Remove code duplication for quiet flag handling
The functionality from the is-terminal crate was lifted into the
standard library.

Since we recently bumped the Rust MSRV, we can now rely on it and remove
a dependency.
@theduke theduke force-pushed the cli-progressbar-fix branch from 2619307 to a98b740 Compare October 30, 2025 17:49
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.

3 participants