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

runsc: Fix newline for cmd usage display #11324

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

vax-r
Copy link

@vax-r vax-r commented Dec 27, 2024

Summary

Originally, some of the commands' usage display didn't achieved to show newline, while some of them does, which are more user-friendly, and won't confuse the message if there're more which should be in the nextline.

Fix this issue by returning usage string with a newline.

Tests

Before the change some commands display their usage like the following, for example runsc start

vax-r@vaxr-ASUSPRO-D840MB-M840MB:~/gvisor$ runsc start
start <container id> - start a secure container.vax-r@vaxr-ASUSPRO-D840MB-M840MB:~/gvisor$

Or runsc wait

vax-r@vaxr-ASUSPRO-D840MB-M840MB:~/gvisor$ runsc wait
wait [flags] <container id>  -checkpoint
    	wait for the next checkpoint to complete
  -pid int
    	select a PID in the container's PID namespace to wait on instead of the container's root process (default -1)
  -rootpid int
    	select a PID in the sandbox root PID namespace to wait on instead of the container's root process (default -1)

We can see in the case of runsc wait, the flag -checkpoint is expected to be shown in a way which is aligned with other flags, like

vax-r@vaxr-ASUSPRO-D840MB-M840MB:~/gvisor$ runsc wait
wait [flags] <container id>  
  -checkpoint
    	wait for the next checkpoint to complete
  -pid int
    	select a PID in the container's PID namespace to wait on instead of the container's root process (default -1)
  -rootpid int
    	select a PID in the sandbox root PID namespace to wait on instead of the container's root process (default -1)

After the change , we can see those commands with this issue are being fixed.

vax-r@vaxr-ASUSPRO-D840MB-M840MB:~/gvisor$ runsc start
start <container id> - start a secure container.
vax-r@vaxr-ASUSPRO-D840MB-M840MB:~/gvisor$
vax-r@vaxr-ASUSPRO-D840MB-M840MB:~/gvisor$ runsc wait
wait [flags] <container id>
  -checkpoint
    	wait for the next checkpoint to complete
  -pid int
    	select a PID in the container's PID namespace to wait on instead of the container's root process (default -1)
  -rootpid int
    	select a PID in the sandbox root PID namespace to wait on instead of the container's root process (default -1)

Originally, some of the commands' usage display didn't achieved to show
newline, while some of them does, which are more user-friendly, and
won't confuse the message if there're more which should be in the
nextline.

Fix this issue by returning usage string with a newline.

Signed-off-by: I Hsin Cheng <[email protected]>
@milantracy
Copy link
Contributor

how about replacing backtick strings with double quota strings, and using the escape character\n for newlines.

for example

diff --git a/runsc/cmd/wait.go b/runsc/cmd/wait.go
index eab74c283..e5bfedf27 100644
--- a/runsc/cmd/wait.go
+++ b/runsc/cmd/wait.go
@@ -51,7 +51,7 @@ func (*Wait) Synopsis() string {
 
 // Usage implements subcommands.Command.Usage.
 func (*Wait) Usage() string {
-       return `wait [flags] <container id>`
+       return "wait [flags] <container id>\n"
 }

@vax-r
Copy link
Author

vax-r commented Dec 27, 2024

how about replacing backtick strings with double quota strings, and using the escape character\n for newlines.

@milantracy -
Thanks for your review !
Yeah that should be better and make the code more understandable, I should modify all the usage string to match the format.

I'll push it up ASAP , thanks !

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.

2 participants