Custom parser implementation- draft #38
Conversation
| @@ -11,6 +11,8 @@ type CommandDefinition struct { | |||
| Example string | |||
| AuthorizationRequired bool | |||
| AuthorizedUsers []string | |||
| AuthorizationFunc func(request Request) bool | |||
| CustomParser func(text string) (*proper.Properties, bool) | |||
There was a problem hiding this comment.
I am not so sure this should be defined here.
It feels to me like you want to be able to use a different implementation than the github.com/shomali11/commander, but the problem is the way it is defined here https://github.com/shomali11/slacker/blob/master/command.go#L19
Seems to me that the best approach is to allow func NewBotCommand(... to accept one more parameter which happens to be an interface with the same methods of the commander library.
This way you can provide your new implementation as a ClientOption in https://github.com/shomali11/slacker/blob/master/slacker.go#L35
Would there ever be a case where you would need different parsers for different commands? I feel like one would be consistent with whatever parser they use throughout the bot.
There was a problem hiding this comment.
I agree. The commander parser provides many functions to the slacker library- more than I expected. I was hoping to find a simple way to override the parser in a sane way. I managed to satisfy the need for the application, but the solution isn't elegant and the time-cost of one is too high for me at the moment :-( Your command is a small study for me...
This is what I ended up using, FYI:
issueRegex = regexp.MustCompile(`^\s*(?:<@(\S+)>)?\s*new\s+"([^"\\]*(?:\\.[^"\\]*)*)"\s+"([^"\\]*(?:\\.[^"\\]*)*)"\s+"([^"\\]*(?:\\.[^"\\]*)*)"\s*$`) // to allow quotation-deliminated arguments
escapeRegex = regexp.MustCompile(`\\(.)`) // to find and remove escaped charactersThere was a problem hiding this comment.
If I understand correctly, you want the regex to support matching sentences within quotations?
If that is the case, we might focus on improving https://github.com/shomali11/commander/blob/master/commander.go#L14
There was a problem hiding this comment.
I will revisit this over the next week or two, likely
|
I will close this until you revisit it |
Re Issue #36
This is a little bit harder/more awkward, maybe unreasonable. Definitely looking for feedback. I pretty much needed this so I made it but...
I didn't want to reimplement the commander entirely XD
I am not as confident that this is acceptable but it can't hurt to offer.