-
Notifications
You must be signed in to change notification settings - Fork 520
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
Enable alias on register command #2581
base: main
Are you sure you want to change the base?
Conversation
cmd/cmd.go
Outdated
m.RegisterCommandByName(name, command) | ||
alias := command.Info().Alias | ||
if alias != "" { | ||
m.RegisterCommandByName(alias, command) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not 100% sure but it could print the command info twice on help message - in different lines, though.
I'd take a better look on how we produce that help message and I'd show the comand + alias in same line - separeted by comma. For example:
app log, app logs Shows log entries for an application
Or maybe register the alias as a hidden command so it won't be generated on the help message even though it works fine when you fire on the command line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are correct, with this changes, help message show twice app log
./bin/tsuru app
App is a program source code running on Tsuru
The following commands are available in the "app" topic:
[...]
app log Shows log entries for an application
app log Shows log entries for an application
[...]
I tried to change where is printed help message, but didn't find.
So I didn't append command alias inside m.topicCommand
, with that change, only show once at help message:
./bin/tsuru app
App is a program source code running on Tsuru
The following commands are available in the "app" topic:
[...]
app list Lists all apps that you have access to
app log Shows log entries for an application
app metadata get Retrieves metadata for an application
[...]
What do you think @nettoclaudio ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My last comment was about when you call tsuru app
to show all available commands.
But this other test, I only call tsuru
and the real command and the alias was in the list, is that correct?!
./bin/tsuru
Client version: dev.
Usage: tsuru command [args]
Available commands:
[...]
app list Lists all apps that you have access to
app log Shows log entries for an application
app logs Shows log entries for an application
app metadata get Retrieves metadata for an application
[...]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm OK on this approach.
To be honest, I don't like to maintain this code on our side anymore. There are a lot of better CLI modules (e.g. spf13/cobra, urfave/cli and so on) than ours. I'm in favor to use them and remove this bunch of code. I don't know if all Tsuru maintainers agree, just my opnion.
68c6ac9
to
c46898f
Compare
Sorry for taking so long to answer. I was trying to write down the aliases at the same line as the main command, I thought we can do it this way: You should change the func (m *Manager) dumpCommands(commands []string) string {
// ...
for _, command := range commands {
// check if command is an alias, if so skip
info := m.Commands[command].Info()
output += formatDescriptionLine(command, info.Alias, info.Description.Desc, maxCmdSize)
}
// ... Then, you should need to change the func formatDescriptionLine(label, alias, description string, maxSize int) string {
// ...
cmds := strings.Join([]string{label, alias}, ", ")
return fmt.Sprintf(fmtStr, strings.ReplaceAll(cmds, "-", " "), description)
} It should produce something like:
|
Thank you @nettoclaudio! I will work on that. |
Close #2576
Enable alias on register new command.
When register a new command, now you can inform
alias
field to generate a new alias.e.g.
tsuru app log
andtsuru app logs
call the same command run.