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.

Sign up for GitHub

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

Added a Llama Picture and Llama Facts Command #196

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Added a Llama Picture and Llama Facts Command #196

Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9636995
Llama Picture Command
Zycrasion May 17, 2023
c63d874
fixed caching issue
Zycrasion May 17, 2023
d25a5ef
now sends the direct links
Zycrasion May 17, 2023
3d44d46
llama fax
Zycrasion May 17, 2023
d80bd0c
Fixed bug and added error checing
Zycrasion May 17, 2023
be5910a
Removed unneeded comments
Zycrasion May 19, 2023
daa6f52
Open Brackets On Same Line as keyword
Zycrasion May 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions lib/Synergy/Reactor/CatPic.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,32 @@ sub _build_reactions ($self, @) {
return $reactions;
}

# Copied cat_pic code
# Because there was no documentation
# Works on terminal, dunno if slack wil be able to display it as an image
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cat pic works by assuming slack will recognise the url and fetch the target for display, so assuming what is returned is a url it should work just fine

# Tried to test it on local slack, but the documentation was pretty bad with the config files
responder llama_pic => {
exclusive => 1, # No idea what this is.
targeted => 1, # Or this
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exclusive is a means to prevent a string being matched by multiple responders, targeted is to do with if you need to target the bot with @botName or PM etc. Someone who has worked on synergy before could answer those questions, but copying what is in catpic is fine here.

help_titles => [ 'llama pic' ],
help => '*llama pic*: get a picture of a llama',
matcher => sub ($text, @) {
return unless $text =~ /\Allama\spic?\z/i;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The regex here is wrong. The version of this in catpic matches "cat" followed optionally by "pic", "jpg", "gif", or "png". And then returns the optional part if it exists.
Regex can be a bit cryptic, but it's really just a very comprehensive pattern matching language, the ? character (the one at the end anyway) marks the previous entity as optional. In catpic this applies to the entire optional match section of space followed by one of the words, in this match the precious entity is the "c" of pic, so this will match either "llama pic" or "llama pi"

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yeah, thanks for pointing that out. I think I have fixed it.

return [];
},
}, sub ($self, $event) {
$event->mark_handled;

my $http_request = $self->hub->http_client->GET(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have kept the $http_future naming from the cat_pic responder here, makes it clearer that it's not just a simple client object and keeps it consistent with cat pic.

"https://llama-as-a-service.vercel.app/llama_url"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you create this API just for this task? cool!

);

return $http_request->on_done(sub($res)
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs some error handling, what happens if we don't get a 200 from the remote api call? What happens if the remote API stops returning a single URL to a picture and starts returning a large html page?

$event->reply($res->content)
});
};

responder cat_pic => {
exclusive => 1,
targeted => 1,
Expand Down