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.

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

template names with _ not correctly parsed #313

Closed
philcerf opened this issue Feb 6, 2021 · 7 comments
Closed

template names with _ not correctly parsed #313

philcerf opened this issue Feb 6, 2021 · 7 comments

Comments

@philcerf
Copy link
Contributor

philcerf commented Feb 6, 2021

Hey.

It seems that template names with an _ are no correctly parsed (at least not from TG) and instead just the portion before the _ gets set in the DB.

Also, the bot replies with an OK for unknown names.

Cheers.

@jfberry
Copy link
Collaborator

jfberry commented Feb 6, 2021

Is this with develop or multilang pr?

@jfberry
Copy link
Collaborator

jfberry commented Feb 6, 2021

Reproduced. Not easy to fix because of the way _ is substituted for a space much earlier in the command processing change. Can't see any easy workaround

@philcerf
Copy link
Contributor Author

philcerf commented Feb 6, 2021

Is this with develop or multilang pr?
that was on develop

Not easy to fix because of the way _ is substituted for a space much earlier in the command processing change. Can't see any easy workaround

Well that's what I tried to tell in the other thread, what is problematic about the current way this mapping is done.
Guess the only way is change the template name in the JSON to use spaces rather than _?

At least we have quite a number of commands that give an OK even though they actually failed.
This is one occasion, I think. Probably a check is needed, whether the template name actually exists.

@jfberry
Copy link
Collaborator

jfberry commented Feb 6, 2021

Of course you can't tell whether the template will still exist, or whether your platform language combination exists.
There are more improvements to the parser needed, have tried in multilang to change the hard coded command matching. Ideally there needs to be different rules for different types of parameters, and quoting etc to work. But that is a bigger re-write moving away from regular expressions.

Will get there eventually, but happy to receive pull requests in the meantime if you want to take it on

@jfberry
Copy link
Collaborator

jfberry commented Feb 6, 2021

See #315

@philcerf
Copy link
Contributor Author

philcerf commented Feb 7, 2021

Replacing _ with " " in the JSON doesn't work btw..

And just for the records,... I've also noted that the template name in the JSON must be all lower case, otherwise it's never matched-

@jfberry
Copy link
Collaborator

jfberry commented Feb 9, 2021

Please take forward in #317

@jfberry jfberry closed this as completed Feb 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants