feat(cli): Added `--color` styling flag to wkg cli by mkatychev · Pull Request #220 · bytecodealliance/wasm-pkg-tools

mkatychev · 2026-06-30T23:01:00Z

Added --color styling flag to wkg cli:

$ ./target/debug/wkg --help
Usage: wkg [OPTIONS] <COMMAND>

Commands:
  ...
Options:
      --color <WHEN>  Controls when to use color [default: auto] [possible values: auto, always, never]
  ...

Added warnln!, statusln!, and helpln! (stolen™ from cargo):

…epracation warning to existing `wkg wit` commands

vados-cosmonic

I'm not against using anstyle for this, but what do you think about using tracing w/ tracing_subscriber::fmt's ANSI color layer? That would give us error/warn/info colors, and be more interoperable with tracing in the libs.

mkatychev · 2026-06-30T23:43:02Z

I'm not against using anstyle for this, but what do you think about using tracing w/ tracing_subscriber::fmt's ANSI color layer? That would give us error/warn/info colors, and be more interoperable with tracing in the libs.

I've thought about that as well, when we get to exposing wkg as a lib, I would rather have switch to tracing as a detection layer similar to how cargo's [Shell::print] does it:
https://github.com/rust-lang/cargo/blob/a595d0da21f228b7fdae64d3d5c0e527ea66bb59/crates/cargo-util-terminal/src/shell.rs#L79-L96

that flexibility needs to be built to and should come with a GlobalContext analogue as well (which we need for better workspace support as well).

I think for now it makes sense to distinguish between purely human stuff (no tracing wrapper) and things triggered by RUST_LOG, ultimately all the anstyle stuff should either be turned off or turned into tracing::event! calls when used as a lib.

vados-cosmonic

Thanks for taking another pass at this, we're almost there.

…tput"

mkatychev · 2026-07-02T05:12:59Z


 [workspace.package]
-edition = "2021"
+edition = "2024"


Tried seeing if bumping would resolve previous test failures:
https://github.com/bytecodealliance/wasm-pkg-tools/actions/runs/28563549597/job/84686098906

Updating edition did not fix issues and this is out of scope but a nice to have regardless.

This was actual fix:

wasm-pkg-tools/crates/wasm-pkg-common/Cargo.toml

Lines 18 to 19 in f5ded81

# running `cargo test` (implicit `--workspace` flag), leaks `ansi-term-output` from wkg

test = []

TLDR: resolver 3 (and 2) bleeds wkg feature flags when running cargo test from the workspace.

cargo test -p wasm-pkg-core is not impacted but it is impractical to invoke explicitly all the time - hence "test" feature flag for ergonomics.

alternative: we can remove the ansi styling completely from the types but regaining functionality (if worth it) will not be feasible short term and the workspace feature bleed will still be a problem for other flags.

To try and restate the problem here -- was it that cargo test -p wkg (or regular cargo test of the workspace) dependency-unifies to wasm-pkg-core *with* ansi-term-outputenabled, no matter what, because of the dep w/ features at thewkg` level?

If so, IMO I don't think we even need to introduce the test feature, adding cargo test -p wasm-pkg-core --no-default-features or something to CI to make sure it runs without is enough.

We could even pull in something like cargo-hack for CI in the future, but my concern here is more that we just get it covered under test so we can't put out a broken release/merge broken code to main.

It's a style thing but what do you think about defining the dep and it's transitive flag dependencies explicitly in wkg too, but as a default?

I believe the syntax would be something like:

[features] defaults = ["ansi-term-output"] ansi-term-output = [ "wasm-pkg-common/ansi-term-output", "wasm-pkg-core/ansi-term-output"]

and then we could make the ansi-term output deps optional like the other lib crates (though they are pulled in by default)

If someone for example wanted to build the binary even smaller, for some reason, they could run cargo install wkg --no-default-features and get a non-ansi-term build this way, rather than the dependencies being required.

I think this pattern is pretty well accepted, as far as optional deps go, even for bin crates, but I'm OK if you don't want to do this either.

vados-cosmonic

LGTM 🚀

vados-cosmonic · 2026-07-02T06:15:29Z


 [workspace.package]
-edition = "2021"
+edition = "2024"


To try and restate the problem here -- was it that cargo test -p wkg (or regular cargo test of the workspace) dependency-unifies to wasm-pkg-core *with* ansi-term-outputenabled, no matter what, because of the dep w/ features at thewkg` level?

If so, IMO I don't think we even need to introduce the test feature, adding cargo test -p wasm-pkg-core --no-default-features or something to CI to make sure it runs without is enough.

We could even pull in something like cargo-hack for CI in the future, but my concern here is more that we just get it covered under test so we can't put out a broken release/merge broken code to main.

It's a style thing but what do you think about defining the dep and it's transitive flag dependencies explicitly in wkg too, but as a default?

I believe the syntax would be something like:

[features] defaults = ["ansi-term-output"] ansi-term-output = [ "wasm-pkg-common/ansi-term-output", "wasm-pkg-core/ansi-term-output"]

and then we could make the ansi-term output deps optional like the other lib crates (though they are pulled in by default)

If someone for example wanted to build the binary even smaller, for some reason, they could run cargo install wkg --no-default-features and get a non-ansi-term build this way, rather than the dependencies being required.

I think this pattern is pretty well accepted, as far as optional deps go, even for bin crates, but I'm OK if you don't want to do this either.

mkatychev · 2026-07-02T13:17:04Z

If so, IMO I don't think we even need to introduce the test feature, adding cargo test -p wasm-pkg-core --no-default-features or something to CI to make sure it runs without is enough.

From a UX perspective, I feel it is important for a default argument set to behave predictably and avoid external dependencies as much as possible (especially for top level subcommands like * build, * check, and * test), it is why this section bothers me so much:

wasm-pkg-tools/crates/wkg/Cargo.toml

Lines 11 to 14 in 23cbf8f

    
           [features] 
        
           default = ["docker-tests"] 
        
           # This feature enables tests that use Docker 
        
           docker-tests = []

I would rather deal with awkward design choices in the margins than rely on callouts & caveats sprinkled everywhere to push the user towards providing a specific set of parameters or lean on external tooling like docker, cargo-hack, bash, or just to scaffold baseline expectations.

This is obviously subjective but a user should be surprised with a failure when they run something like cargo test on a repo after a fresh clone.

mkatychev · 2026-07-02T13:34:21Z

        into_auth_should_read_docker_registry_credentials();
        into_auth_should_other_registry_credentials();
-        std::env::remove_var("DOCKER_CONFIG");
+        // NOT-SAFE: this is likely causing race conditions


I run into race conditions on darwin arm very often with these tests, @ryan-surname-p have you had the same experience?

I don't recall having issues with this but that's the fun thing about race conditions I could have just gotten lucky.

That said this is definitely racy which it seems someone has acknowledged above on line 235 as well.

Wrapping it in unsafe does make it much more clear.

the unsafe was a hard error thanks to the 2024 edition bump!

vados-cosmonic · 2026-07-02T13:45:39Z

Yeah unfortunately I think how Rust work has our hands tied here, given these facts:

Using the ansi-term-output feature in wkg pulls in extra crates
ansi-term-output isn't actually required, but we want it to be enabled by default

I'd like to have this be really obvious from reading the Cargo.toml.

Separately, the docker-tests feature should not exist (IMO, like the test feature shouldn't) -- the most elegant way I've seen that navigated is discovering those features in build.rs and injecting a dynamic cfg or ENV that lets the tests do what they need to do. I think the current code was an initial approach at this that should be removed.

As far as what the user (developer) tests/has to configure, there is no actual change here based on what I'm describing, it's purely notational, except for the user losing the ability to build without those features required. Requiring those features without giving an opt out is the same as having it as a default feature, except slightly worse because the user has no way to do a more minimal build.

The onus to test both the default build and the minimal build is on us as maintainers IMO -- the average developer need not do it.

mkatychev · 2026-07-02T14:00:44Z

@vados-cosmonic thanks for the explanation on #220 (comment)

vados-cosmonic · 2026-07-02T14:20:14Z

@mkatychev Oh no worries, the build.rs feature detection thing is it's a pattern I've seen others do and shamlessly stole 😆

It's funny how every time something just-barely-uncomfortable like this comes up I go search on rust-lang/rust and find some 5+ year old issue where someone is asking for the same thing :)

mkatychev · 2026-07-02T14:22:37Z

I think I fell for the classic scope creep trap in this PR, I will revert the core/common changes and merge without the display impls and submit a separate PR

Added `--color` styling flag to wkg cli: ```sh-session $ ./target/debug/wkg --help Usage: wkg [OPTIONS] <COMMAND> Commands: ... Options: --color <WHEN> Controls when to use color [default: auto] [possible values: auto, always, never] ... ``` * Added `warnln!`, `statusln!`, and `helpln!` macros

Mikhail Katychev and others added 4 commits June 30, 2026 16:43

refactor(cli): move wkg wit <cmd> subcommands to wkg <cmd>, add d…

d6d91c4

…epracation warning to existing `wkg wit` commands

feat(cli): add support for --color flag

b055755

feat(cli): add warnln! and statusln! macros to wkg

3b09976

feat(cli): add helpln! to wkg

6b578f2

mkatychev requested a review from vados-cosmonic June 30, 2026 23:01

This comment was marked as outdated.

Sign in to view

mkatychev changed the title ~~Mkatychev/wkg add colour~~ feat(cli): Added --color styling flag to wkg cli Jun 30, 2026

vados-cosmonic reviewed Jun 30, 2026

View reviewed changes

mkatychev changed the base branch from mkatychev/refactor/move-wkg-wit-top to main June 30, 2026 23:44

feat(core): add ansi styling in Display of PackageRef and PackageSpec

3572fe5

mkatychev force-pushed the mkatychev/wkg-add-colour branch from d96658a to 3572fe5 Compare June 30, 2026 23:48

vados-cosmonic reviewed Jul 1, 2026

View reviewed changes

Comment thread crates/wasm-pkg-common/src/package.rs Outdated

Comment thread crates/wasm-pkg-common/src/package.rs Outdated

Comment thread crates/wasm-pkg-core/src/resolver.rs Outdated

Comment thread crates/wkg/src/main.rs

mkatychev added 2 commits July 1, 2026 13:11

refactor(core): feature gate anstyle

ff425f3

refactor(cli): use core and common with anstyle feature flag

8569654

mkatychev force-pushed the mkatychev/wkg-add-colour branch from 273bd2e to 8569654 Compare July 1, 2026 18:28

Merge branch 'main' into mkatychev/wkg-add-colour

a349be5

mkatychev commented Jul 1, 2026

View reviewed changes

Comment thread crates/wkg/src/main.rs

mkatychev force-pushed the mkatychev/wkg-add-colour branch from 7751a71 to 467ac5d Compare July 1, 2026 18:33

mkatychev added 2 commits July 1, 2026 13:36

chore(config): run cargo-autoinherit

82379d5

style(config): sort workspace dependencies

d620cc7

mkatychev force-pushed the mkatychev/wkg-add-colour branch from 467ac5d to d620cc7 Compare July 1, 2026 18:36

mkatychev requested a review from vados-cosmonic July 1, 2026 18:37

fix(vis): moved ansi mod items to pub(crate)

8654209

vados-cosmonic reviewed Jul 2, 2026

View reviewed changes

Comment thread crates/wasm-pkg-core/Cargo.toml Outdated

Comment thread crates/wkg/src/main.rs

Comment thread crates/wkg/src/main.rs

mkatychev requested a review from vados-cosmonic July 2, 2026 01:30

mkatychev added 3 commits July 1, 2026 22:27

refactor(core,common): rename "anstyle" feature flag to "ansi-term-ou…

91c5d4f

…tput"

refactor(common): remove uses of is_terminal

cbc3fea

fmt

4cdb501

mkatychev commented Jul 2, 2026

View reviewed changes

refactor: update to rust 2024, add test feature flag

0fe596d

mkatychev force-pushed the mkatychev/wkg-add-colour branch from f5ded81 to 0fe596d Compare July 2, 2026 05:29

vados-cosmonic approved these changes Jul 2, 2026

View reviewed changes

mkatychev mentioned this pull request Jul 2, 2026

wasm-pkg-client: validate semver compliance on publish #213

Merged

mkatychev commented Jul 2, 2026

View reviewed changes

Merge branch 'main' into mkatychev/wkg-add-colour

2609470

revert(core,common): defer ansi impl Display impls

34a8116

mkatychev mentioned this pull request Jul 2, 2026

Remove docker-tests issue #223

Open

chore: add anstream test flag to wkg dev

f909263

mkatychev merged commit 03dfe9b into main Jul 2, 2026
6 checks passed

mkatychev deleted the mkatychev/wkg-add-colour branch July 2, 2026 14:52

	# running `cargo test` (implicit `--workspace` flag), leaks `ansi-term-output` from wkg
	test = []

Uh oh!

Conversation

mkatychev commented Jun 30, 2026

Uh oh!

This comment was marked as outdated.

vados-cosmonic left a comment

Choose a reason for hiding this comment

Uh oh!

mkatychev commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vados-cosmonic left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mkatychev Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mkatychev Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vados-cosmonic Jul 2, 2026

Choose a reason for hiding this comment

Uh oh!

vados-cosmonic left a comment

Choose a reason for hiding this comment

Uh oh!

vados-cosmonic Jul 2, 2026

Choose a reason for hiding this comment

Uh oh!

mkatychev commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mkatychev Jul 2, 2026

Choose a reason for hiding this comment

Uh oh!

ryan-surname-p Jul 2, 2026

Choose a reason for hiding this comment

Uh oh!

mkatychev Jul 2, 2026

Choose a reason for hiding this comment

Uh oh!

vados-cosmonic commented Jul 2, 2026

Uh oh!

mkatychev commented Jul 2, 2026

Uh oh!

vados-cosmonic commented Jul 2, 2026

Uh oh!

mkatychev commented Jul 2, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mkatychev commented Jun 30, 2026 •

edited

Loading

mkatychev Jul 2, 2026 •

edited

Loading

mkatychev Jul 2, 2026 •

edited

Loading

mkatychev commented Jul 2, 2026 •

edited

Loading