add support for directly send msg to room; example in README #10
Conversation
There was a problem hiding this comment.
Thanks for implementing this @C1tas! Left some comments below, but I didn't try out the code yet. Maybe @punchagan is up for testing these changes? (I haven't run errbot in quite a while, and it'd take a long time to set up).
zulip.py
Outdated
|
|
||
| def send_message(self, msg): | ||
| super().send_message(msg) | ||
| # import pdb ; pdb.set_trace() |
There was a problem hiding this comment.
This is an artifact from debugging, should be removed.
zulip.py
Outdated
|
|
||
| def extract_identifiers_from_string(self, text): | ||
| """ | ||
There was a problem hiding this comment.
What's the purpose of this empty docstring?
zulip.py
Outdated
| if len(re_res.groups()) > 0: | ||
| username = re_res[1] | ||
|
|
||
| # import pdb ; pdb.set_trace() |
There was a problem hiding this comment.
Another debugging artifact.
| emails=[txtrep], | ||
| client=self.client) | ||
|
|
||
| log.debug('building an identifier from %s.', txtrep) |
There was a problem hiding this comment.
Another debugging artifact.
There was a problem hiding this comment.
(Or do other backends actually log something here?)
zulip.py
Outdated
| username, streamname, topicname = self.extract_identifiers_from_string(txtrep) | ||
|
|
||
| if username: | ||
|
|
README.md
Outdated
|
|
||
| * Rooms in ErrBot are streams in Zulip. | ||
|
|
||
| #### directly send msg to room |
There was a problem hiding this comment.
In user-facing documentation, words like "message" shouldn't be abbreviated. It'd also be nice to have a paragraph or two explaining what goes into the double brackets, and when you would want to "directly send msg to room"
| if self.check_email(text): | ||
| username = text | ||
|
|
||
| if text[0] == "#": |
There was a problem hiding this comment.
The logic in this if-statement is rather complicated. Perhaps there is a simpler way to parse this? Maybe something that involves extracting every occurrence of {{}} and making the if-statements switch based on the number of these statements.
There was a problem hiding this comment.
sry for my poor regex skill, if you have some cool idea, just instead of using my poor code
README.md
Outdated
| self.send( | ||
| self.build_identifier("#{{code_runtime_alert}}*{{hello}}"), | ||
| "test" | ||
| ) |
There was a problem hiding this comment.
If we expect the send method to be called directly by users, is it worth adding regexes and complex logic to the build_identifier method? We could just have a function that creates the appropriate ZulipRoom instance given the stream and topic?
There was a problem hiding this comment.
I think it depend on user. And the reason why i use function send like this is that i found err-bot official docs using code like this.
There was a problem hiding this comment.
Fair enough. I just felt it would be easier to have stream and topic as separate arguments, but if this is the documented way of doing it, then so be it.
|
Hi, is there any chance of this getting merged? |
#9