Skip to content

Escape markdown in faulty source commands#1205

Merged
Akarys42 merged 4 commits into
python-discord:masterfrom
soumitradev:source_escape_markdown
Oct 2, 2020
Merged

Escape markdown in faulty source commands#1205
Akarys42 merged 4 commits into
python-discord:masterfrom
soumitradev:source_escape_markdown

Conversation

@soumitradev
Copy link
Copy Markdown
Contributor

Closes #1177

@soumitradev soumitradev requested a review from a team as a code owner October 1, 2020 19:21
@soumitradev soumitradev requested review from GhostofGoes and SebastiaanZ and removed request for a team October 1, 2020 19:21
@ghost ghost added the needs 2 approvals label Oct 1, 2020
@ghost
Copy link
Copy Markdown

ghost commented Oct 1, 2020

Thank you for contributing to Python Discord!

Please check out the following documents:

I had flake8 turned off in my dpy env -_-
Copy link
Copy Markdown
Contributor

@kwzrd kwzrd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This approach does not seem to be functional:

image

In order to allow backticks in an in-line markdown codeblock, I believe the appropriate approach is to use double-backticks, e.g.:

image

This produces:

image

I'm not sure how others would feel about this, but we could also just not use a codeblock to repeat the input. Using the current approach of escape_markdown is then functional:

image

Comment thread bot/exts/info/source.py
@ghost ghost added s: waiting for author Waiting for author to address a review or respond to a comment and removed needs 2 approvals labels Oct 2, 2020
@kwzrd kwzrd added a: information Related to information commands: (doc, help, information, reddit, site, tags) p: 3 - low Low Priority t: bug Something isn't working labels Oct 2, 2020
@soumitradev
Copy link
Copy Markdown
Contributor Author

If we use double backticks it will again break if the argument provided contains a double backtick.

I agree with getting rid of the codeblock itself

I'll commit that for now, if others have an issue, we can discuss further.

Double backtick will break if argument contains a double backtick, so getting rid of the codeblock itself makes more sense in my opionion.

Also fix the style issue with multiline string by storing the escaped arg in another variable
@ghost ghost added needs 2 approvals and removed s: waiting for author Waiting for author to address a review or respond to a comment labels Oct 2, 2020
@soumitradev soumitradev requested a review from kwzrd October 2, 2020 12:25
Copy link
Copy Markdown
Contributor

@kwzrd kwzrd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am personally ok with this solution. Thanks for the contribution!

Copy link
Copy Markdown
Contributor

@Akarys42 Akarys42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution!

@ghost ghost removed the needs 1 approval label Oct 2, 2020
@Akarys42 Akarys42 merged commit 1353454 into python-discord:master Oct 2, 2020
@soumitradev
Copy link
Copy Markdown
Contributor Author

Can this PR be marked as Low Effort or Spam? I'm not very happy with the work I did.

@soumitradev soumitradev deleted the source_escape_markdown branch October 3, 2020 18:03
@scragly
Copy link
Copy Markdown
Contributor

scragly commented Oct 4, 2020

While it is a relatively simple contribution, our team don't consider it low effort or spam as it's directly addressing an issue ticket we wanted to be resolved. Your solution wasn't super involved because it didn't need to be, and that's a good thing. As we appreciate the contribution, there's no reason for us to mark it invalid.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: information Related to information commands: (doc, help, information, reddit, site, tags) p: 3 - low Low Priority t: bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: ` chars are not escaped when parsing !source

4 participants