Include run in list of common commands#1752
Conversation
Summary of ChangesHello @CrimsonVoid, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the user experience by making the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds the run command to the list of common commands to improve its discoverability. The change in src/cmd/utils.go is correct. However, the system tests in test/system/002-help.bats were not fully updated to account for the new line in the help output, which will cause them to fail. I've left comments with suggestions to fix the test assertions.
|
Build succeeded. ✔️ unit-test SUCCESS in 2m 08s |
Signed-off-by: CrimsonVoid <CrimsonVoid@users.noreply.github.com>
Signed-off-by: CrimsonVoid <CrimsonVoid@users.noreply.github.com>
Signed-off-by: CrimsonVoid <CrimsonVoid@users.noreply.github.com>
CrimsonVoid
force-pushed
the
doc-run-commonCmd
branch
from
744ec71 to
0f1e88a
Compare
|
Build succeeded. ✔️ unit-test SUCCESS in 2m 09s |
debarshiray
left a comment
debarshiray
left a comment
There was a problem hiding this comment.
Thanks for working on this, @CrimsonVoid , and my apologies for the delay!
For a while, I was absolutely sure that someone else had requested or submitted something like this in the past, but I failed to find an issue or a pull request. So let's chalk it up to my failing memory. :)
Could you please correct your Git authorship information with your full name and a real email address, unless you are worried about privacy? We need a real full name and email address for tracking copyright holders in case a licensing issue crops up.
The Signed-off-by trailer has no meaning without a real full name and email address. For what it's worth, projects as diverse as GCC, GnuPG, Linux, Moby and Podman don't allow anonymous or pseudonymous contributions.
I am curious what you use the |
| var builder strings.Builder | ||
| builder.WriteString("create Create a new Toolbx container\n") | ||
| builder.WriteString("enter Enter an existing Toolbx container\n") | ||
| builder.WriteString("run Run a command in an existing Toolbx container\n") |
There was a problem hiding this comment.
I am a little hesitant to include too many things in commonCommands at the risk of users glazing over the output (like myself). A short list of three commands covering most people's workflow might be a sweet spot and moving discoverability of toolbox run et al. somewhere else might be more prudent; perhaps just mentioning a full list of commands can be found elsewhere would be enough.
It turns out that when we rewrote Toolbx from POSIX shell to Go, we introduced a subtle regression in this area by losing a hint that there are other commands than just these three. :)
The output from the POSIX shell implementation used to be:
$ toolbox
toolbox: missing command
These are some common commands:
create Create a new toolbox container
enter Enter an existing toolbox container
list List all existing toolbox containers and images
Try 'toolbox --help' for more information.
The output from the Go implementation is:
$ toolbox
Error: missing command
create Create a new Toolbx container
enter Enter an existing Toolbx container
list List all existing Toolbx containers and images
Run 'toolbox --help' for usage.
Notice how the "These are some common commands:" line disappeared. As far as I can make out, it happened in commit aaf81a56ea607da0.
Then in commit 40fc1689a3d90f7, we made the help command and --help option work without man(1), and as part of that re-introduced something similar. See the "Common commands are:" line below, assuming man(1) is absent:
$ toolbox help
toolbox - Tool for interactive command line environments on Linux
Common commands are:
create Create a new toolbox container
enter Enter an existing toolbox container
list List all existing toolbox containers and images
Go to https://containertoolbx.org/ for further information.
Looking around at some common command line tools, I see that Git has something similar:
$ git
...
These are common Git commands used in various situations:
start a working area (see also: git help tutorial)
clone Clone a repository into a new directory
init Create an empty Git repository or reinitialize an existing one
...
'git help -a' and 'git help -g' list available subcommands and some
concept guides. See 'git help <command>' or 'git help <concept>'
to read about a specific subcommand or concept.
See 'git help git' for an overview of the system.I am leaning towards restoring the hint that we lost as a way to imply to readers that there's more than these three. I don't know if it should be the "These are some common commands" string that we used to have or the "Common commands are" string that we use now. I will leave that to you to decide.
What do you think?
2 participants
The idea was to make
toolbox runa little more discoverable when running justtoolbox; for a while I was under the assumption that toolbox only supported create, enter, and list (knowing about run earlier would have solved the major reason I reached for distrobox)I know the help document covers all available commands, but it was busy enough that I glazed over the commands section at the end.
Tradeoffs & Concerns
I am a little hesitant to include too many things in commonCommands at the risk of users glazing over the output (like myself). A short list of three commands covering most people's workflow might be a sweet spot and moving discoverability of
toolbox runet al. somewhere else might be more prudent; perhaps just mentioning a full list of commands can be found elsewhere would be enough.