Closed JTeeuwissen closed 3 years ago
I marked it as draft to avoid any accidental merges, but I am not planning to modify the merge without feedback.
What bothers me the most is the syntax
/run python bash
is just confusing. What is python bash.
Maybe require a ->
or something.
Also the regex is a little over engineered the only thing you have to do is to take the language part of the regex (?: +(?P<language>\S*)\s*|\s*)
copy paste it immediately after the language part (before the args part) and change the name - done.
r'/(?:edit_last_)?run(?: +(?P<language>\S*)\s*|\s*)(?: +(?P<output_syntax>\S*)\s*|\s*)(?:\n(?P<args>(?:[^\n\r\f\v]*\n)*?)\s*|\s*)```(?:(?P<syntax>\S+)\n\s*|\s*)(?P<source>.*)```(?:\n?(?P<stdin>(?:[^\n\r\f\v]\n?)+)+|)'
I added an -> to the syntax, and tried to indicate this in the readme.
the only thing you have to do
That is exactly what I did, except I had to remove the \s*
part of language as it matched the whitespace of output_syntax, and if I removed this, the output language would match the next line. But adding a required arrow in the syntax fixed that problem so I added back the \s*
.
I worked a bit more on the regexes and came up with these:
self.run_regex_code = re.compile(
r'(?s)/(?:edit_last_)?run'
r'(?: +(?P<language>\S*?)\s*|\s*)'
r'(?:-> *(?P<output_syntax>\S*)\s*|\s*)'
r'(?:\n(?P<args>(?:[^\n\r\f\v]*\n)*?)\s*|\s*)'
r'```(?:(?P<syntax>\S+)\n\s*|\s*)(?P<source>.*)```'
r'(?:\n?(?P<stdin>(?:[^\n\r\f\v]\n?)+)+|)'
)
self.run_regex_file = re.compile(
r'/run(?: *(?P<language>\S*)\s*?|\s*?)?'
r'(?: *-> *(?P<output>\S*)\s*?|\s*?)?'
r'(?:\n+(?P<args>(?:[^\n\r\f\v]+\n?)*)\s*|\s*)?'
r'(?:\n*(?P<stdin>(?:[^\n\r\f\v]\n?)+)+|)?'
)
This allows any number (and even omission) of spaces around the ->
in the normal version
The "file version" is still a bit wanky but it works for now.
Also there is no reason to hard fail when an output language is unknown Discord will not care if you give it something invalid as a syntax so we should neither. You can take out all the "valid output syntax" checking code and the dict and just write the chosen output language into the response without caring if it will work or not.
That was fast.
Looks good to me - at least it seems to not break anything existing.
Before I add that, could you rebase -i
everything down into 1 commit?
I think i've got it.
56
This pr is untested and probably contains some bugs. If anyone has the time to look at it / fix it, they would be more than welcome.