Skip to content

Conversation

Zabuzard
Copy link
Member

@Zabuzard Zabuzard commented Nov 19, 2021

Bug

Fixes #269 .

Basically, /tag-manage raw did not escape triple backticks correctly.

issue

Resulting in crap like this:

tag

Fix

The fix applied here is relatively simple, we still use the existing escaping utility but apply a patch on top where we replace its result with the proper result.

I slightly rewrote the code section to make it clearer that we now have a replace before and one after the escape utility. Otherwise its too easy to confuse with two replacements happening before, i.e.

escape(text.replace(...)).replace(...)
// vs
escape(text.replace(...).replace(...))

I also adjusted the existing test cases and added a new unit test to cover this particular complex case as well.

* it didnt escape ``` correctly (\``` instead of \`\`\`)
* adjusted test cases
* added a new test case to cover this (and other more complex situations)
@Zabuzard Zabuzard added bug Something isn't working priority: major labels Nov 19, 2021
@Zabuzard Zabuzard added this to the Migrate existing commands milestone Nov 19, 2021
@Zabuzard Zabuzard self-assigned this Nov 19, 2021
@Zabuzard Zabuzard requested review from a team as code owners November 19, 2021 12:33
@Zabuzard Zabuzard enabled auto-merge (rebase) November 19, 2021 12:33
@Zabuzard Zabuzard linked an issue Nov 19, 2021 that may be closed by this pull request
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@java-coding-prodigy
Copy link
Contributor

Could you share a screenshot of the output after fixing the bug?

@Zabuzard
Copy link
Member Author

Could you share a screenshot of the output after fixing the bug?

Sure:

after fix

@Zabuzard
Copy link
Member Author

Zabuzard commented Nov 19, 2021

That said, I spot another bug. Embeds seem to mess up leading spaces (they are part of the message though).

It doesnt seem like this is fixable though...

Copy link
Contributor

@marko-radosavljevic marko-radosavljevic left a comment

Choose a reason for hiding this comment

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

Tests failed to catch this, and if I reviewed this code (and tests) I wouldn't catch it either. Just something to note. ^^

@Zabuzard
Copy link
Member Author

Been a week since the last change. Got one approval by marko and one semi approval by illu (I guess). Merging.

@Zabuzard Zabuzard disabled auto-merge November 25, 2021 07:53
@Zabuzard Zabuzard merged commit 3b94879 into develop Nov 25, 2021
@Zabuzard Zabuzard deleted the bugfix/tag_raw_code_snippets branch November 25, 2021 07:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority: major
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tag raw doesnt work with code snippets
4 participants