Skip to content
This repository has been archived by the owner on Jun 22, 2020. It is now read-only.

add global command flag to alias add #3

Closed
wants to merge 3 commits into from

Conversation

miketheman
Copy link
Collaborator

Works with exisiting aliases and extends the command syntax to capture
the global flag and pass that along to AliasedCommand.

Updates add_alias_route to make the decision on whether it should match
a command or message listener.

Not working yet: after restarting process, load_aliases does not register
global aliases, as the state is not persisted.

@miketheman
Copy link
Collaborator Author

I have the desire to have commands be executed even when not directed at the bot (command: true).

I could have started modifying the AliasStore class to enable the persistence level, but wanted to discuss the namespacing of the code as laid out before doing that.

Currently, the "path" of the main handler is Lita::Alias::ChatHandler, whereas according to the plugin authoring guide this should be Lita::Handlers::Alias - this has the benefit of conforming to most other plugin layouts, as well as providing some built-in logic around namespaces.

Changing this in code isn't a big deal, but it will affect persisted data, as the key path today is "lita:handlers:chat_handler:aliases" and this would change to "lita:handlers:alias".

This might open up the ability to store each alias as it's own key->hash, instead of all keys living inside one hget key.

Something like:

127.0.0.1:6379> HMSET "lita:handlers:alias:alice" name alice command 'Welcome to my restaurant!' global false
OK
127.0.0.1:6379> HGETALL "lita:handlers:alias:alice"
1) "name"
2) "alice"
3) "command"
4) "Welcome to my restaurant!"
5) "global"
6) "false"
127.0.0.1:6379> HMGET "lita:handlers:alias:alice" name command global
1) "alice"
2) "Welcome to my restaurant!"
3) "false"

These all live inside the scope of the Handler's namespace, so calling redis.keys returns only "lita:handlers:alias:* - somewhat simplifying the code.

I think this is a longer topic, probably out of scope for this change, but wanted to kick it off with @apsoto .

@miketheman
Copy link
Collaborator Author

(Also, @apsoto - no idea why a travis build wasn't triggered for the pull request - that's very odd.)

Works with exisiting aliases and extends the command syntax to capture
the global flag and pass that along to AliasedCommand.

Updates `add_alias_route` to make the decision on whether it should match
a command or message listener.

Not working yet: after restarting process, load_aliases does not register
global aliases, as the state is not persisted.
Discovered with mutant testing, bringing this class to 100% free of
mutants!

It's unlikely that we would ever generate an AliasedCommand without a
name attribute, but if it ever happened, this test would uncover that.
@apsoto
Copy link
Owner

apsoto commented Jul 3, 2015

I made the conscious decision to namespace things that way because there was more than just a single handler class. Lita::Handlers::AliasedCommand and Lita::Handlers::AliasStore did not seem like logical names to me. Just a design decision, unless it's causing problems, I'd rather leave it as is.

@apsoto
Copy link
Owner

apsoto commented Jul 3, 2015

In regards to the global flag. Can you show me some examples of how you would use it? I personally haven't come across the need for triggering things without a command. Since it's an alias, my thinking is that you want to explicitly to trigger it.

@miketheman
Copy link
Collaborator Author

Re: namespacing, sure thing, wanted to float the concept by you. I don't think there's any problem there.

For global, with Slack adapters we cannot use the / shortcut for commands, so we end up typing out the bot's name for directed commands.

Our current practices using hubot's alias scripts allowed for global aliases, so we evolved to using commands like DEPLOYTIME that then trigger another command.

@apsoto
Copy link
Owner

apsoto commented Jul 5, 2015

My first thought is whether to make this a config value (config.alias.use_command = true|false), to make the alias use a command or instead use an option (--global) when adding the alias.

Let me run this against a slack room so I can get a feel for your use case.

@apsoto
Copy link
Owner

apsoto commented Jul 6, 2015

I tried with slack, and I used '!' as the alias and it worked executing the aliases without having to address the bot user.

config.robot.alias = '!'

Does that help solve things for you?

@miketheman
Copy link
Collaborator Author

@apsoto I believe that will probably solve the concern, and will not introduce added complexity to the plugin.

I'll go ahead and close this one, and pull in the added test case (05e78e6) if you don't mind.

@miketheman miketheman closed this Jul 6, 2015
@miketheman miketheman deleted the miketheman/global branch July 6, 2015 17:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants