Open wojpawlik opened 7 years ago
TelegramBot#onText() is a primitive way to match text messages, primitive enough that I do not propose any approach that builds on top of it. It is meant to beginners and simple use cases.
We are trying to build a more comprehensive command handler. This means we shall be depending on the "text"
event, as we listen for text messages and process them. Therefore, our command handler will be incompatible with TelegramBot#onText().
I suggest for an approach that does not rely on numerous regular expressions in the matching. We only one regexp to match and extract the following components of a text message:
/
help
@my_bot
arg1 arg2
The prefixes to be used will be determined during construction of the instance, as an option e.g. options.tgfancy.commands.prefixes
(an array of prefixes e.g. ['/', '#', '!']
).
Also, we would need to determine the bot's name (with TelegramBot#getMe()). Before the operation completes, no polling/webhooking (:laugh:) should be happening. This should avoid the case where we can not process text messages, since the method has failed. We do not want to lose any updates, whatsoever! When the operation successfully completes, polling/webhooking should be initiated!
A regexp can be constructed using new RegExp()
, by constructing a string like so:
const prefixes = ["/", "!"].join("");
const bot_name = "my_bot"; // retrieved earlier!
const regexp_string = `^[${prefixes}](\w+)(@${bot_name})?(\s.+)?$`;
const regexp = new RegExp(regexp_string);
resulting in a regexp, like /^[!/](\w+)(@my_bot)?(\s.+)?$/
.
Using this single regexp, we are able to match the right commands. We are assured it will only match with the correct prefixes. It will match commands without the bot's name. But if a command includes the bot's name, if it is the correct one, it will be matched.
Therefore, internally, we will use a preferable data structure to reserve mapping between commands and callbacks (registered with Tgfancy#onCommand()). This data structure can be chosen to provide better search costs, for example, using binary search, etc. Such a mapping can be visualized like so:
const mapping = {
"help": cb1,
"ping": cb2,
// ... more ...
};
That is the general idea. Let's work from there!
Thanks for response @GochoMugo.
I don't see any advantage in not building on top of onText
, maybe except that it allows us to get a fresh start. Could you elaborate reason for that suggestion?
About no fetching updates before getMe
finishes, currently impossible, as they're initialized in the constructor; unless you have a better idea or someone else implements it, I think I'll implement and PR to both ntba and tgfancy my proposal 6 about it, I think that'd break nothing.
preferable data structure to reserve mapping between commands and callbacks
Hashmap sounds like a perfect match here. do we use object or Map
?
I agree with other stuff proposed by you, maybe I think I'd implement some things slightly differently, but those are details, we'll see when we'll have some code.
I don't see any advantage in not building on top of onText, maybe except that it allows us to get a fresh start. Could you elaborate reason for that suggestion?
TelegramBot#onText() is a primitive way to match text messages, primitive enough that I do not propose any approach that builds on top of it. It is meant to beginners and simple use cases.
We are trying to build a more comprehensive command handler. This means we shall be depending on the "text" event, as we listen for text messages and process them. Therefore, our command handler will be incompatible with TelegramBot#onText().
I suggest for an approach that does not rely on numerous regular expressions in the matching. We only need one regexp...
How I see onCommand
We implement 2 functions,
onCommand
andonCommands
, one in term of other.onCommand(command: String, callback: Function)
onCommands(commands: [String], callback: Function)
Now I saw
_.castArray
, using it we could haveonCommand
accept both string and array of strings, what do you think?I think they should be synchronized with
onText
and should respectonlyFirstMatch
; all of my proposals futfill that.I see 3 possible implementations:
1. For eachonCommands
call, we create regex and useonText
to register it.~~Pros: + requires no changes to n-t-b-a~~
~~Cons: - At least one regex per
onCommands
call; linear searching~~onText
handler. We pass a regex which matches everything that looks like a command, something like:Then we use command => callback mapping (object or Map) to get callback we need in amortized constant time.
Problem with it, is that it cannot be currently implemented to work correctly with
onlyFirstMatch
. I think that the right thing to do when there is no callback for command, is to proceed to nextonText
callback, and there is currently no way to do that. When regex matches, no further callbacks are executed, there is no way for a function to tell "yes, it did match, but that doesn't mean I can handle it".We'd just have special constant, that when returned from
onText
callback would mean just thatIt shouldn't be too hard to implement tho, I thought about it before, and I have another use-case, we'd just need to change L112-114 in telegram.js to something like
Pros: +
O(1)
+ should be quite simple to implement actuallyevery proposal is+ my personal favouritenot sure anymoreCons: - requires adding a feature to n-t-b-a
RegExp
(ie. implementingexec
method) which would do all the logic which I talked in point 2 inside itself, we'd pass noop as callback. Theexec
method of the object would just return something falsey when there is no callback registered for a command, which is is interpeted as no match, and causes nextonText
RegExp
-like to be checked.Pros: +
O(1)
+ should be quite simple to implement actuallyevery proposal is +puts almost all of the logic into single object (?) + requires no changes to n-t-b-aCons: - hack?
4. Mix of 2 and 3, we have RegExp-like object, which returns function to call and the match object, we call the function with the match object as argument from the
onText
callback.+
O(1) \+puts almost all of the logic into single object (?)
+ requires no changes to n-t-b-a- hack?
Getting username
That's the problem with writing this.
We can of course require the username in
options
, but... that probably wouldn't feel very well, when we can fetch it fromgetMe
and add 0 configuration.The problem is, it's an async command, and thus, because we set up getting updates immiedetly, in the constructor, and we may end up getting messages to process before we know the username, before we're prepared to handle them.
Solutions?
Make(I don't like this one anymore)processUpdate
async (that'd change it's signature) -- store a promise in the bot object, initially it'd bePromise.resolve()
, makeprocessUpdate
schedule processing new updates when the promise is resolved. When callingonCommand
for the first time, we'd then dopromise = Promise.join(this.getMe, promise)
.listen
(or some other name) and have the constructor call it by default, but provide option to opt-out from calling it in the c-tor, and maybe option toonCommand
to call it whengetMe
resolvesJust ignore the username and handle all commands untilgetMe
resolvesJust ignore the username and handle all commands untilgetMe
resolvesThat method requires changes to n-t-b-a, and it's hard to explain in words, so here's pseudocode:
Do you have other ideas?
Which approaches should we take?