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

Add some decoration to tool CLI #4865

Merged
merged 1 commit into from
Jul 8, 2024
Merged

Add some decoration to tool CLI #4865

merged 1 commit into from
Jul 8, 2024

Conversation

charliermarsh
Copy link
Member

Mostly small things. I added entrypoint counts and bolded the executable names.

Closes #4815.

@charliermarsh charliermarsh requested a review from zanieb July 7, 2024 20:16
@charliermarsh charliermarsh added cli Related to the command line interface preview Experimental behavior labels Jul 7, 2024
writeln!(
printer.stderr(),
"Uninstalled: {}",
"Uninstalled {} executable{s}: {}",
Copy link
Member Author

Choose a reason for hiding this comment

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

Are you opposed to using "executable" for user-facing copy?

Copy link
Member

Choose a reason for hiding this comment

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

It feels a little weird, I don't have any great alternatives though. "tool entry points"? eh.

Copy link
Member

Choose a reason for hiding this comment

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

We use tool entry points everywhere else in this output, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

But entry point is not as general... We also support data scripts which aren't entry points. I feel like entry point is sort of an implementation detail.

Copy link
Member

Choose a reason for hiding this comment

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

Fair. Why not just "commands" then?

Copy link
Member

Choose a reason for hiding this comment

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

I don't have strong feelings here.

Copy link
Member

Choose a reason for hiding this comment

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

Cargo uses executable, e.g.

   Replacing /Users/zb/.cargo/bin/cargo-nextest
    Replaced package `cargo-nextest v0.9.70` with `cargo-nextest v0.9.72` (executable `cargo-nextest`)

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, I'm gonna run with executable. I can change other user-facing copy too for consistency.

@charliermarsh
Copy link
Member Author

(I will update fixtures once approved, just a bit tedious.)

Comment on lines 268 to 269
"Installing tool entry points into {}",
"Installing tool entrypoints into: {}",
Copy link
Member

Choose a reason for hiding this comment

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

These are always two words in the official documentation

https://packaging.python.org/en/latest/specifications/entry-points/

Copy link
Member

Choose a reason for hiding this comment

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

(I like how entrypoints looks too, but I think we should be consistent. I've been following the official docs here)

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh strange, thank you, I can change it.

@charliermarsh charliermarsh force-pushed the charlie/tool-cli branch 4 times, most recently from 3db5614 to 7507786 Compare July 8, 2024 13:09
@charliermarsh charliermarsh merged commit 947cfa1 into main Jul 8, 2024
49 checks passed
@charliermarsh charliermarsh deleted the charlie/tool-cli branch July 8, 2024 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli Related to the command line interface preview Experimental behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

uv tool should use some colors in the CLI
2 participants