-
Notifications
You must be signed in to change notification settings - Fork 48
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
💥 Command Aliases #155
base: master
Are you sure you want to change the base?
💥 Command Aliases #155
Conversation
Rebased onto master so compare view is much cleaner now. |
and other solution would be a Wrapper for a command -
public struct Alias<ClientError: Error>: CommandProtocol {
// [...]
fileprivate init<C: CommandProtocol>(_ command: C, alias: String) where C.ClientError == ClientError {
verb = alias
function = "alias for '${command.verb}': {$command.function}"
run = command.run
usage = command.usage
}
} usage: registry.register(Alias(ListCommand(), alias: "L")) drawback: the alias strings are not defined at the command class |
is it not possible to make the alias as optional, so that it not need to add and it is not a breaking change? |
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.
changes to display alias in commands list
@@ -96,6 +99,10 @@ public final class CommandRegistry<ClientError: Error> { | |||
{ | |||
for command in commands { | |||
commandsByVerb[command.verb] = CommandWrapper(command) | |||
// Register command for each additional alias | |||
for alias in command.aliases { | |||
commandsByVerb[alias] = CommandWrapper(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.
commandsByVerb[alias] = CommandWrapper(command) | |
let wrappedCommand = CommandWrapper(command) | |
wrappedCommand.verb = alias | |
wrappedCommand.function = "alias for '${command.verb}': {$command.function}" | |
commandsByVerb[alias] = wrappedCommand |
@@ -20,6 +20,9 @@ public protocol CommandProtocol { | |||
/// `help`). | |||
var verb: String { get } | |||
|
|||
/// Optional list of additional verbs which can be used to invoke this command. | |||
var aliases: [String] { get } |
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.
var aliases: [String] { get } | |
var aliases: [String]? { get } |
but let aliases be optional and not defined (null)
PS:
need to add
extension CommandProtocol {
var aliases: [String]? { return nil }
}
or do just a default implementation:
extension CommandProtocol {
var aliases: [String] { return [] }
}
``
Implements #153.
Opening this PR as a draft for discussion. I've got a simple/dumb implementation of aliases working in mas and you can see the behavior below. The
install
command was given an alias ofi
, but thehelp
output lists theinstall
command twice.This was achieved by:
CommandProtocol.aliases
Modifying the
CommandProtocol
is a breaking change and each user of this library would need to add thealias
property to every command, even if the command has no aliases. The 2nd item causes the command duplication but could be improved by extending the idea of an alias into the registry and auto-generating the help description with something like:Perhaps a better approach is to register aliases differently instead of modifying the model. Going to think some more about this but welcome any feedback or suggestions.